bug#31138: Native json slower than json.el

bug#31138: Native json slower than json.el

[Sebastien Chapuis]

[-- Attachment #1: Type: text/plain, Size: 789 bytes --]


Hi,

I have tested the new function json-parse-string to use it in lsp-mode
and was suprised when I found out that this function is slower than
json-read-from-string from json.el:

Test with a string of 4042446 characters:
```
json-parse-string:       9,166010 seconds
json-read-from-string:   6.028625 seconds
```

I took a look at the code, and if I remove the call to
code_convert_string in json_make_string (see the attached diff),
it's way faster:

Test with a string of 4042411 characters
```
json-parse-string (without code_convert_string):
0.505582 seconds
```

Is this conversion really necessary ? Should I set some variable to avoid
this ?


By the way, what is the state of this patch:
https://lists.gnu.org/archive/html/emacs-devel/2017-12/msg00916.html
Could it be merge ?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-patch, Size: 534 bytes --]

diff --git a/src/json.c b/src/json.c
index b046d34f66..1fc976214d 100644
--- a/src/json.c
+++ b/src/json.c
@@ -219,8 +219,7 @@ json_has_suffix (const char *string, const char *suffix)
 static Lisp_Object
 json_make_string (const char *data, ptrdiff_t size)
 {
-  return code_convert_string (make_specified_string (data, -1, size, false),
-                              Qutf_8_unix, Qt, false, true, true);
+  return make_specified_string (data, -1, size, false);
 }
 
 /* Create a multibyte Lisp string from the null-terminated UTF-8

[-- Attachment #3: Type: text/plain, Size: 22 bytes --]


--
Sebastien Chapuis

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: Sebastien Chapuis <sebastien@chapu.is>
> Date: Thu, 12 Apr 2018 21:13:41 +0200
> 
> I took a look at the code, and if I remove the call to
> code_convert_string in json_make_string (see the attached diff),
> it's way faster:
> 
> Test with a string of 4042411 characters
> ```
> json-parse-string (without code_convert_string):
> 0.505582 seconds
> ```
> 
> Is this conversion really necessary ?

Yes, it's necessary, because the input string may include raw bytes,
which will crash Emacs if not handled properly.

I'm surprised that the slowdown due to the conversion is so large,
though.  It doesn't feel right, even with a 4MB string.

bug#31138: Native json slower than json.el

[Sebastien Chapuis]

Eli Zaretskii writes:

> I'm surprised that the slowdown due to the conversion is so large,
> though.  It doesn't feel right, even with a 4MB string.

I've digged a bit to know why it is so slow, and I've found that if I'm
wrapping `json-parse-string` with a `with-temp-buffer`, it is now way
faster:

results of benchmark-run with a string of 4043212 characters
```
(with-temp-buffer (json-parse-string str)):
(0.814315554 1 0.11941178500000005)

(json-parse-string str):
(11.542233167 1 0.14954429599999997)

(with-temp-buffer (json-read-from-string str)):
(5.9781185610000005 29 4.967349412000001)

(json-read-from-string str):
(5.601267 24 4.723292248000001)
```

Any idea why ? The current (buffer-size) is 1063954, if it is relevant.

> Yes, it's necessary, because the input string may include raw bytes,
> which will crash Emacs if not handled properly.

The Jansson documentation guarantee that the strings returned
from the library are always UTF-8 encoded [1].
By knowing that guarantee, is it possible to reconsider the use of
code_convert_string ?
Encoding a string to UTF-8 which is already UTF-8 encoded seems
useless..


[1] https://jansson.readthedocs.io/en/2.11/apiref.html#c.json_string_value

--
Sebastien Chapuis

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: Sebastien Chapuis <sebastien@chapu.is>
> Cc: 31138@debbugs.gnu.org
> Date: Sun, 15 Apr 2018 16:40:18 +0200
> 
> 
> > I'm surprised that the slowdown due to the conversion is so large,
> > though.  It doesn't feel right, even with a 4MB string.
> 
> I've digged a bit to know why it is so slow, and I've found that if I'm
> wrapping `json-parse-string` with a `with-temp-buffer`, it is now way
> faster:
> 
> results of benchmark-run with a string of 4043212 characters
> ```
> (with-temp-buffer (json-parse-string str)):
> (0.814315554 1 0.11941178500000005)
> 
> (json-parse-string str):
> (11.542233167 1 0.14954429599999997)
> 
> (with-temp-buffer (json-read-from-string str)):
> (5.9781185610000005 29 4.967349412000001)
> 
> (json-read-from-string str):
> (5.601267 24 4.723292248000001)
> ```

Interesting.

> Any idea why ?

Where did str come from?  Did you insert it into the buffer or
something?  Could that explain the difference in performance?

More generally, can you post the string you are using for the
benchmarking, and the benchmark code as well?  That would make the
discussion less abstract.

> > Yes, it's necessary, because the input string may include raw bytes,
> > which will crash Emacs if not handled properly.
> 
> The Jansson documentation guarantee that the strings returned
> from the library are always UTF-8 encoded [1].

You assume that the library has no bugs, yes?  Because if it does,
then we might crash Emacs by trusting it so much.  Letting invalid
bytes creep into Emacs buffers and strings is a sure recipe for an
eventual crash.

> By knowing that guarantee, is it possible to reconsider the use of
> code_convert_string ?

Since it's already much faster than a Lisp implementation, why would
we want to risk crashing an Emacs session by omitting the decoding?

> Encoding a string to UTF-8 which is already UTF-8 encoded seems
> useless..

It's decoding, not encoding, and the process of decoding examines
every sequence in the byte stream and ensures they are valid UTF-8.

Emacs never trusts any external data to be what the user or Lisp tell
it is; I see no reason why we should make an exception in this
particular case.

Thanks.

bug#31138: Native json slower than json.el

[Sébastien Chapuis]

Hello,

I tried to find the cause of this but still without any success.
Here is a reproducible case:

You can download the json file at:
https://gist.githubusercontent.com/yyoncho/dec968b69185305ed02741e18b27a82d/raw/334b0a51bc52cc3c98edb8ff4bccb5fc4531842b/large.json

Open the file with `emacs -Q large.json`.
Switch to the scratch buffer and run:

```
(with-current-buffer  "large.json"
  (benchmark-run 10 (json-parse-string (buffer-string))))
;;; (2.5371836119999998 10 0.111044641)

(with-current-buffer  "large.json"
  (let ((str (buffer-string)))
    (benchmark-run 10 (with-temp-buffer (json-parse-string str)))))
;;; (1.510604359 10 0.13192760000000003)

(with-current-buffer  "large.json"
  (let ((str (buffer-string)))
    (benchmark-run 10 (with-temp-buffer (json-read-from-string str)))))
;;; (1.970248228 114 1.058150570000001)
```

Thanks,
Sebastien Chapuis

Le dim. 15 avr. 2018 à 23:19, Eli Zaretskii <eliz@gnu.org> a écrit :
>
> > From: Sebastien Chapuis <sebastien@chapu.is>
> > Cc: 31138@debbugs.gnu.org
> > Date: Sun, 15 Apr 2018 16:40:18 +0200
> >
> >
> > > I'm surprised that the slowdown due to the conversion is so large,
> > > though.  It doesn't feel right, even with a 4MB string.
> >
> > I've digged a bit to know why it is so slow, and I've found that if I'm
> > wrapping `json-parse-string` with a `with-temp-buffer`, it is now way
> > faster:
> >
> > results of benchmark-run with a string of 4043212 characters
> > ```
> > (with-temp-buffer (json-parse-string str)):
> > (0.814315554 1 0.11941178500000005)
> >
> > (json-parse-string str):
> > (11.542233167 1 0.14954429599999997)
> >
> > (with-temp-buffer (json-read-from-string str)):
> > (5.9781185610000005 29 4.967349412000001)
> >
> > (json-read-from-string str):
> > (5.601267 24 4.723292248000001)
> > ```
>
> Interesting.
>
> > Any idea why ?
>
> Where did str come from?  Did you insert it into the buffer or
> something?  Could that explain the difference in performance?
>
> More generally, can you post the string you are using for the
> benchmarking, and the benchmark code as well?  That would make the
> discussion less abstract.
>
> > > Yes, it's necessary, because the input string may include raw bytes,
> > > which will crash Emacs if not handled properly.
> >
> > The Jansson documentation guarantee that the strings returned
> > from the library are always UTF-8 encoded [1].
>
> You assume that the library has no bugs, yes?  Because if it does,
> then we might crash Emacs by trusting it so much.  Letting invalid
> bytes creep into Emacs buffers and strings is a sure recipe for an
> eventual crash.
>
> > By knowing that guarantee, is it possible to reconsider the use of
> > code_convert_string ?
>
> Since it's already much faster than a Lisp implementation, why would
> we want to risk crashing an Emacs session by omitting the decoding?
>
> > Encoding a string to UTF-8 which is already UTF-8 encoded seems
> > useless..
>
> It's decoding, not encoding, and the process of decoding examines
> every sequence in the byte stream and ensures they are valid UTF-8.
>
> Emacs never trusts any external data to be what the user or Lisp tell
> it is; I see no reason why we should make an exception in this
> particular case.
>
> Thanks.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: Sébastien Chapuis <sebastien@chapu.is>
> Date: Sat, 23 Mar 2019 09:59:23 +0800
> Cc: 31138@debbugs.gnu.org, yyoncho@gmail.com
> 
> I tried to find the cause of this but still without any success.
> Here is a reproducible case:
> 
> You can download the json file at:
> https://gist.githubusercontent.com/yyoncho/dec968b69185305ed02741e18b27a82d/raw/334b0a51bc52cc3c98edb8ff4bccb5fc4531842b/large.json
> 
> Open the file with `emacs -Q large.json`.
> Switch to the scratch buffer and run:
> 
> ```
> (with-current-buffer  "large.json"
>   (benchmark-run 10 (json-parse-string (buffer-string))))
> ;;; (2.5371836119999998 10 0.111044641)
> 
> (with-current-buffer  "large.json"
>   (let ((str (buffer-string)))
>     (benchmark-run 10 (with-temp-buffer (json-parse-string str)))))
> ;;; (1.510604359 10 0.13192760000000003)
> 
> (with-current-buffer  "large.json"
>   (let ((str (buffer-string)))
>     (benchmark-run 10 (with-temp-buffer (json-read-from-string str)))))
> ;;; (1.970248228 114 1.058150570000001)
> ```

Is this with the latest Emacs master branch?  If not, can you try the
latest master?

FWIW, I cannot reproduce your results with the current master.  I get
the following times:

  (with-current-buffer  "large.json"
    (let ((str (buffer-string)))
      (benchmark-run 10 (with-temp-buffer (json-parse-string str)))))
  (0.78125 10 0.15625)

  (with-current-buffer  "large.json"
    (benchmark-run 10
      (progn (goto-char (point-min))
	     (json-parse-buffer))))
  (0.890625 10 0.140625)

  (with-current-buffer  "large.json"
    (let ((str (buffer-string)))
      (benchmark-run 10 (with-temp-buffer (json-read-from-string str)))))
  (2.84375 126 1.21875)

So on my system the native JSON support does this job about 4 times
faster than the Lisp implementation.  I see the same ratio of 4
consistently in both optimized and unoptimized builds of Emacs (the
above numbers are from an optimized build).

Please note that in my experiments I visited the large.json file with
find-file-literally, not with find-file.  In my testing, I didn't see
any difference in timings either way, but visiting literally is more
correct for the real-life use cases.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Date: Sat, 23 Mar 2019 10:15:26 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: yyoncho@gmail.com, 31138@debbugs.gnu.org
> 
> So on my system the native JSON support does this job about 4 times
> faster than the Lisp implementation.  I see the same ratio of 4
> consistently in both optimized and unoptimized builds of Emacs (the
> above numbers are from an optimized build).

I've just tried on another system, and there I get the native JSON
support 3 times faster than json.el.

bug#31138: Native json slower than json.el

[Sébastien Chapuis]

Sorry for not being clear, my main concern was the difference when
wrapping json-parse-string with with-temp-buffer or not.

You are right, I tested with the current master branch and it is
faster (emacs -Q):

(with-current-buffer  "large.json"
  (benchmark-run 10 (json-parse-string (buffer-string))))
;;; (1.45898128 10 0.15294547200000003)

(with-current-buffer  "large.json"
  (let ((str (buffer-string)))
    (benchmark-run 10 (with-temp-buffer (json-parse-string str)))))
;;; (0.706171416 10 0.18795709700000002)

(with-current-buffer  "large.json"
  (let ((str (buffer-string)))
    (benchmark-run 10 (with-temp-buffer (json-read-from-string str)))))
;;; (2.476727624 138 1.5660531400000006)

I have tested to read the file literally, as you suggested and there
is now no difference with or without with-temp-buffer (emacs -Q):

(with-current-buffer (find-file-noselect "large.json" nil t)
  (benchmark-run 10 (json-parse-string (buffer-string))))
;;; (0.7011264119999999 10 0.118889765)

(with-current-buffer  (find-file-noselect "large.json" nil t)
  (let ((str (buffer-string)))
    (benchmark-run 10 (with-temp-buffer (json-parse-string str)))))
;;; (0.7159130309999999 10 0.15112133999999997)

For the context, with lsp-mode, we have users complaining about
performance of json-parse-string [1].
Some have found out that the function is way faster with emacs -Q but
it is "dead slow" with their Spacemacs setup.

In lsp-mode, we are reading child process output by calling
`make-process` with `:coding 'no-conversion`, I think it has the same
behavior than reading a file "literally" ?
Is there anything else we could do to improve the performance from
reading the process output ?

Now the problem is how can we have the same performance with a regular
emacs setup than with emacs -Q ?
With my emacs setup, I have this result:

(with-current-buffer (find-file-noselect "large.json" nil t)
  (benchmark-run 10 (json-parse-string (buffer-string))))
;;; (1.515018996 10 0.5256668049999996)

(with-current-buffer (find-file-noselect "large.json" nil t)
  (let ((str (buffer-string)))
    (benchmark-run 10 (with-temp-buffer (json-parse-string str)))))
;;; (1.156755376 10 0.5596599300000001)

This is almost 2x slower than with emacs -Q.
Note that there is still a difference with and without `with-temp-buffer`.
What can we do to reach emacs -Q performance ?

Thanks for your time,
Sebastien Chapuis

[1] https://github.com/emacs-lsp/lsp-mode/issues/210#issuecomment-471314904

Le sam. 23 mars 2019 à 17:40, Eli Zaretskii <eliz@gnu.org> a écrit :
>
> > Date: Sat, 23 Mar 2019 10:15:26 +0200
> > From: Eli Zaretskii <eliz@gnu.org>
> > Cc: yyoncho@gmail.com, 31138@debbugs.gnu.org
> >
> > So on my system the native JSON support does this job about 4 times
> > faster than the Lisp implementation.  I see the same ratio of 4
> > consistently in both optimized and unoptimized builds of Emacs (the
> > above numbers are from an optimized build).
>
> I've just tried on another system, and there I get the native JSON
> support 3 times faster than json.el.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: Sébastien Chapuis <sebastien@chapu.is>
> Date: Sat, 23 Mar 2019 20:59:46 +0800
> Cc: Ivan Yonchovski <yyoncho@gmail.com>, 31138@debbugs.gnu.org
> 
> Sorry for not being clear, my main concern was the difference when
> wrapping json-parse-string with with-temp-buffer or not.

Ah, okay.  I thought the concern was with the native JSON support
being slower than json.el.

> You are right, I tested with the current master branch and it is
> faster (emacs -Q):
> 
> (with-current-buffer  "large.json"
>   (benchmark-run 10 (json-parse-string (buffer-string))))
> ;;; (1.45898128 10 0.15294547200000003)
> 
> (with-current-buffer  "large.json"
>   (let ((str (buffer-string)))
>     (benchmark-run 10 (with-temp-buffer (json-parse-string str)))))
> ;;; (0.706171416 10 0.18795709700000002)
> 
> (with-current-buffer  "large.json"
>   (let ((str (buffer-string)))
>     (benchmark-run 10 (with-temp-buffer (json-read-from-string str)))))
> ;;; (2.476727624 138 1.5660531400000006)

OK, this is consistent with what I see: about 3- to 4-fold speedup
from using the native JSON support.

> I have tested to read the file literally, as you suggested and there
> is now no difference with or without with-temp-buffer (emacs -Q):
> 
> (with-current-buffer (find-file-noselect "large.json" nil t)
>   (benchmark-run 10 (json-parse-string (buffer-string))))
> ;;; (0.7011264119999999 10 0.118889765)
> 
> (with-current-buffer  (find-file-noselect "large.json" nil t)
>   (let ((str (buffer-string)))
>     (benchmark-run 10 (with-temp-buffer (json-parse-string str)))))
> ;;; (0.7159130309999999 10 0.15112133999999997)

I didn't mean find-file-noselect, I meant find-file-literally.  Which
one did you use in your testing?

> For the context, with lsp-mode, we have users complaining about
> performance of json-parse-string [1].
> Some have found out that the function is way faster with emacs -Q but
> it is "dead slow" with their Spacemacs setup.
> 
> In lsp-mode, we are reading child process output by calling
> `make-process` with `:coding 'no-conversion`, I think it has the same
> behavior than reading a file "literally" ?

Yes.

> Is there anything else we could do to improve the performance from
> reading the process output ?

I don't know yet, let's first try to solve the issue below:

> Now the problem is how can we have the same performance with a regular
> emacs setup than with emacs -Q ?
> With my emacs setup, I have this result:
> 
> (with-current-buffer (find-file-noselect "large.json" nil t)
>   (benchmark-run 10 (json-parse-string (buffer-string))))
> ;;; (1.515018996 10 0.5256668049999996)
> 
> (with-current-buffer (find-file-noselect "large.json" nil t)
>   (let ((str (buffer-string)))
>     (benchmark-run 10 (with-temp-buffer (json-parse-string str)))))
> ;;; (1.156755376 10 0.5596599300000001)
> 
> This is almost 2x slower than with emacs -Q.
> Note that there is still a difference with and without `with-temp-buffer`.
> What can we do to reach emacs -Q performance ?

I think the only sure way is to find the customization(s) which are
responsible for the slowdown.  First, use find-file-literally, and if
that doesn't help, bisect your customizations to find which one(s)
cause this two-fold slowdown.

Once you identify the customizations responsible for the slowdown, we
could then try to figure out whether that is due to some bugs or not,
and if the latter, then how to avoid the slowdown even with those
customizations in effect.

Thanks.

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 4377 bytes --]

Hi Eli,

Take a look at the following piece of code:

https://gist.github.com/yyoncho/9e9c4e14734fdd9a22d6600a88a27ae1
(with the latest emacs compiled from master)

Unfortunately, I wasn't unable to reproduce the behaviour with "emacs -q"
and even it does not reproduce right after I load my emacs configuration
but after doing some navigation/coding. I believe that native parsing is
calling some function list defined in the elisp space but I am unable to
track down this. Please, let me know what I can do to help to diagnose this
issue.

Thanks,
Ivan

On Sat, Mar 23, 2019 at 3:21 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Sébastien Chapuis <sebastien@chapu.is>
> > Date: Sat, 23 Mar 2019 20:59:46 +0800
> > Cc: Ivan Yonchovski <yyoncho@gmail.com>, 31138@debbugs.gnu.org
> >
> > Sorry for not being clear, my main concern was the difference when
> > wrapping json-parse-string with with-temp-buffer or not.
>
> Ah, okay.  I thought the concern was with the native JSON support
> being slower than json.el.
>
> > You are right, I tested with the current master branch and it is
> > faster (emacs -Q):
> >
> > (with-current-buffer  "large.json"
> >   (benchmark-run 10 (json-parse-string (buffer-string))))
> > ;;; (1.45898128 10 0.15294547200000003)
> >
> > (with-current-buffer  "large.json"
> >   (let ((str (buffer-string)))
> >     (benchmark-run 10 (with-temp-buffer (json-parse-string str)))))
> > ;;; (0.706171416 10 0.18795709700000002)
> >
> > (with-current-buffer  "large.json"
> >   (let ((str (buffer-string)))
> >     (benchmark-run 10 (with-temp-buffer (json-read-from-string str)))))
> > ;;; (2.476727624 138 1.5660531400000006)
>
> OK, this is consistent with what I see: about 3- to 4-fold speedup
> from using the native JSON support.
>
> > I have tested to read the file literally, as you suggested and there
> > is now no difference with or without with-temp-buffer (emacs -Q):
> >
> > (with-current-buffer (find-file-noselect "large.json" nil t)
> >   (benchmark-run 10 (json-parse-string (buffer-string))))
> > ;;; (0.7011264119999999 10 0.118889765)
> >
> > (with-current-buffer  (find-file-noselect "large.json" nil t)
> >   (let ((str (buffer-string)))
> >     (benchmark-run 10 (with-temp-buffer (json-parse-string str)))))
> > ;;; (0.7159130309999999 10 0.15112133999999997)
>
> I didn't mean find-file-noselect, I meant find-file-literally.  Which
> one did you use in your testing?
>
> > For the context, with lsp-mode, we have users complaining about
> > performance of json-parse-string [1].
> > Some have found out that the function is way faster with emacs -Q but
> > it is "dead slow" with their Spacemacs setup.
> >
> > In lsp-mode, we are reading child process output by calling
> > `make-process` with `:coding 'no-conversion`, I think it has the same
> > behavior than reading a file "literally" ?
>
> Yes.
>
> > Is there anything else we could do to improve the performance from
> > reading the process output ?
>
> I don't know yet, let's first try to solve the issue below:
>
> > Now the problem is how can we have the same performance with a regular
> > emacs setup than with emacs -Q ?
> > With my emacs setup, I have this result:
> >
> > (with-current-buffer (find-file-noselect "large.json" nil t)
> >   (benchmark-run 10 (json-parse-string (buffer-string))))
> > ;;; (1.515018996 10 0.5256668049999996)
> >
> > (with-current-buffer (find-file-noselect "large.json" nil t)
> >   (let ((str (buffer-string)))
> >     (benchmark-run 10 (with-temp-buffer (json-parse-string str)))))
> > ;;; (1.156755376 10 0.5596599300000001)
> >
> > This is almost 2x slower than with emacs -Q.
> > Note that there is still a difference with and without
> `with-temp-buffer`.
> > What can we do to reach emacs -Q performance ?
>
> I think the only sure way is to find the customization(s) which are
> responsible for the slowdown.  First, use find-file-literally, and if
> that doesn't help, bisect your customizations to find which one(s)
> cause this two-fold slowdown.
>
> Once you identify the customizations responsible for the slowdown, we
> could then try to figure out whether that is due to some bugs or not,
> and if the latter, then how to avoid the slowdown even with those
> customizations in effect.
>
> Thanks.
>

[-- Attachment #2: Type: text/html, Size: 5571 bytes --]

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Sat, 23 Mar 2019 15:31:39 +0200
> Cc: Sébastien Chapuis <sebastien@chapu.is>, 
> 	31138@debbugs.gnu.org
> 
> Take a look at the following piece of code:
> 
> https://gist.github.com/yyoncho/9e9c4e14734fdd9a22d6600a88a27ae1
> (with the latest emacs compiled from master)

OK.

> Unfortunately, I wasn't unable to reproduce the behaviour with "emacs -q" and even it does not reproduce
> right after I load my emacs configuration but after doing some navigation/coding.

Sorry, I don't think I understand what behavior you were unable to
reproduce.  Please elaborate.

> I believe that native parsing is
> calling some function list defined in the elisp space but I am unable to track down this.

Maybe so, but I don't think I see a Lisp function called from json.c code.

> Please, let me know what I can do to help to diagnose this issue.

Are we still talking about too-slow performance of json-parse-string?
If so, and if you think some Lisp code is responsible for that, then
running your benchmark test under profiler.el would be the first thing
I'd suggest.

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 2292 bytes --]

Hi Eli,

Sorry for being unclear:

Here it is a summary of the gist from the previous mail is doing:

1. Prepare json string to parse
2. Set counter my/call-counter to 0
3. Create function my/test-query-function which will increase the counter.
4. Add this function to 'kill-buffer-query-functions.
5. Parse the string from 1). using json-parse-string
6. At this point, my/call-counter is 32982 which means that
json-parse-string has triggered 32982 calls to my/test-query-function . I
would add also that the number of strings in the json file that I am using
is exactly 32982 so I suspect that the issue is related to json_make_string
.

Here it is the output of from single invocation of json-parse-string -
https://gist.github.com/yyoncho/101a87260b407d9f327b24c72ab15a92 - as you
can see there are numerous elisp functions invoked under json-parse-string.

What I am unable to provide is a minimal example so you could reproduce
this behaviour on your side using emacs -Q.

Thanks,
Ivan

On Sat, Mar 23, 2019 at 4:00 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: yyoncho <yyoncho@gmail.com>
> > Date: Sat, 23 Mar 2019 15:31:39 +0200
> > Cc: Sébastien Chapuis <sebastien@chapu.is>,
> >       31138@debbugs.gnu.org
> >
> > Take a look at the following piece of code:
> >
> > https://gist.github.com/yyoncho/9e9c4e14734fdd9a22d6600a88a27ae1
> > (with the latest emacs compiled from master)
>
> OK.
>
> > Unfortunately, I wasn't unable to reproduce the behaviour with "emacs
> -q" and even it does not reproduce
> > right after I load my emacs configuration but after doing some
> navigation/coding.
>
> Sorry, I don't think I understand what behavior you were unable to
> reproduce.  Please elaborate.
>
> > I believe that native parsing is
> > calling some function list defined in the elisp space but I am unable to
> track down this.
>
> Maybe so, but I don't think I see a Lisp function called from json.c code.
>
> > Please, let me know what I can do to help to diagnose this issue.
>
> Are we still talking about too-slow performance of json-parse-string?
> If so, and if you think some Lisp code is responsible for that, then
> running your benchmark test under profiler.el would be the first thing
> I'd suggest.
>

[-- Attachment #2: Type: text/html, Size: 3339 bytes --]

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Sat, 23 Mar 2019 16:32:48 +0200
> Cc: Sébastien Chapuis <sebastien@chapu.is>, 
> 	31138@debbugs.gnu.org
> 
> 6. At this point, my/call-counter is 32982 which means that json-parse-string has triggered 32982 calls to
> my/test-query-function . I would add also that the number of strings in the json file that I am using is exactly
> 32982 so I suspect that the issue is related to json_make_string .

OK, but why is that a problem?

> Here it is the output of from single invocation of json-parse-string -
> https://gist.github.com/yyoncho/101a87260b407d9f327b24c72ab15a92 - as you can see there are numerous
> elisp functions invoked under json-parse-string. 

What I see in the profile is this:

  . json-parse-string takes the lion's share of the CPU time, which is
    expected;
  . some byte-compiled function takes another significant portion of
    CPU time; to see what function is that, load the relevant Lisp
    code from a .el file, instead of from .elc file, then re-run the
    profile

> What I am unable to provide is a minimal example so you could reproduce this behaviour on your side using
> emacs -Q. 

Actually, I'm not yet sure what to reproduce.  I'm probably missing
something.

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 1850 bytes --]

Hi Eli,

> OK, but why is that a problem?

It makes native json parsing slower than the elisp parsing and it is
unclear why do we get these calls in the performance critical section of
the code(at least for lsp-mode) and it is unclear how to avoid them.
Without such recipe or a code fix native json parsing will be unusable in
combination with any package that attaches yet to be defined extension
point.

Thanks,
Ivan

On Sat, Mar 23, 2019 at 4:55 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: yyoncho <yyoncho@gmail.com>
> > Date: Sat, 23 Mar 2019 16:32:48 +0200
> > Cc: Sébastien Chapuis <sebastien@chapu.is>,
> >       31138@debbugs.gnu.org
> >
> > 6. At this point, my/call-counter is 32982 which means that
> json-parse-string has triggered 32982 calls to
> > my/test-query-function . I would add also that the number of strings in
> the json file that I am using is exactly
> > 32982 so I suspect that the issue is related to json_make_string .
>
> OK, but why is that a problem?
>
> > Here it is the output of from single invocation of json-parse-string -
> > https://gist.github.com/yyoncho/101a87260b407d9f327b24c72ab15a92 - as
> you can see there are numerous
> > elisp functions invoked under json-parse-string.
>
> What I see in the profile is this:
>
>   . json-parse-string takes the lion's share of the CPU time, which is
>     expected;
>   . some byte-compiled function takes another significant portion of
>     CPU time; to see what function is that, load the relevant Lisp
>     code from a .el file, instead of from .elc file, then re-run the
>     profile
>
> > What I am unable to provide is a minimal example so you could reproduce
> this behaviour on your side using
> > emacs -Q.
>
> Actually, I'm not yet sure what to reproduce.  I'm probably missing
> something.
>

[-- Attachment #2: Type: text/html, Size: 2612 bytes --]

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Sat, 23 Mar 2019 17:27:58 +0200
> Cc: Sébastien Chapuis <sebastien@chapu.is>, 
> 	31138@debbugs.gnu.org
> 
> > OK, but why is that a problem?
> 
> It makes native json parsing slower than the elisp parsing and it is unclear why do we get these calls in the
> performance critical section of the code(at least for lsp-mode) and it is unclear how to avoid them. Without
> such recipe or a code fix native json parsing will be unusable in combination with any package that attaches
> yet to be defined extension point. 

Sorry, I don't think I understand the issue.  Currently, the native
JSON parsing is between 3 and 4 times faster than the Lisp
implementation, as shown by my benchmarks posted here, which were
confirmed by Sébastien.  So it cannot be unusable, not more so than
the Lisp implementation.  Am I missing something?

As for avoiding the calls to json_make_string: before we discuss that,
we should at least establish that those calls eat up a significant
percentage of the CPU time in the use cases that are of interest.  I
don't think we have established that yet; the large number of calls to
that function doesn't by itself mean it consumes a significant portion
of CPU time.

And I'm now confused what is the issue we are investigating here.
Originally, it seemed like the problem was the native JSON processing
being slower than the Lisp implementation.  Then we've established
that the native implementation is 3 to 4 times faster, and the problem
was that in "emacs -Q" it is twice faster than in OP's customized
Emacs session; I suggested a way to try to find the reason(s) for that
slowdown.  Now it seems like you are saying that json_make_string
could be the problem, and the rest of the issue is not really
interesting?  If so, my suggestion would be to run Emacs under 'perf'
and see which parts of json-parse-string take most of the CPU time,
then we'd have solid basis for discussing the significance of the
calls to json_make_string in this scenario.  If you want me to do the
'perf' run (with the caveat that in "emacs -Q" it might run faster
than in your customized Emacs), please give a complete recipe, and I
will do it.

And I still think you should run the profile with *.el files, to see
what is that compiled function which took a significant percentage of
processing time in the profile you showed.

Thanks.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Sat, 23 Mar 2019 20:03:29 +0200
> 
>  Sorry, I don't think I understand the issue.  Currently, the native
>  JSON parsing is between 3 and 4 times faster than the Lisp
>  implementation, as shown by my benchmarks posted here, which were
>  confirmed by Sébastien.  So it cannot be unusable, not more so than
>  the Lisp implementation.  Am I missing something?
> 
> You asked why native json parsing calling "kill-buffer-query-functions" for each string transformation is a
> problem - my answer to that is that if we do not find a way to fix that native json parsing will not be usable for
> Elisp developer since it will get slower depending on external factors.
> 
>  As for avoiding the calls to json_make_string: before we discuss that,
>  we should at least establish that those calls eat up a significant
>  percentage of the CPU time in the use cases that are of interest.
> 
> I do not want to avoid calls to json_make_string but I want json_make_string to be changed so it does not
> trigger "kill-buffer-query-functions".

Ah, okay.  Thanks for explaining the problem to me, I now understand
what's bothering you.

However, I cannot reproduce what you describe, not with the latest
Emacs master branch.  When I repeat the recipe you posted in

  https://gist.github.com/yyoncho/9e9c4e14734fdd9a22d6600a88a27ae1

I get zero as the final value of the counter, not 32982.  As I'd
expect, as I don't see why kill-buffer-query-functions would be called
in this scenario.  This was in "emacs -Q", do you get a non-zero count
even in "emacs -Q"?

To see why kill-buffer-query-functions is invoked in your case, I
suggest the following:

  . Modify the above recipe to call 'message' inside
    my/test-query-function.
  . Run Emacs under GDB after putting a breakpoint in Fmessage (this
    is the C name of 'message').
  . Run your recipe.  When the breakpoint in 'message' breaks, display
    the Lisp backtrace using the "xbacktrace" command (it is defined
    in src/.gdbinit in the Emacs source tree).

Alternatively (and maybe easier for you), set debug-on-entry to
trigger when my/test-query-function is called, then run your recipe,
and look at the backtrace to see why it was called.

I hope that either of these two ways will allow you to find the code
which is responsible for calling kill-buffer-query-functions when JSON
string is being parsed.  Once we understand that, we could see how to
avoid it.

>  If so, my suggestion would be to run Emacs under 'perf'
>  and see which parts of json-parse-string take most of the CPU time,
>  then we'd have solid basis for discussing the significance of the
>  calls to json_make_string in this scenario.  If you want me to do the
>  'perf' run (with the caveat that in "emacs -Q" it might run faster
>  than in your customized Emacs), please give a complete recipe, and I
>  will do it.
> 
> Like I said in the first mail - I cannot give you the recipe since I am not able to track down what is causing that
> slowdown and I need help on that. Can you give me a link on how to run emacs under "perf"?

Here are a few:

  http://www.brendangregg.com/perf.html
  https://perf.wiki.kernel.org/index.php/Tutorial#Sampling_with_perf_record
  https://perf.wiki.kernel.org/index.php/Tutorial#Sample_analysis_with_perf_report

The perf man pages are also useful.

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 2647 bytes --]

Hi Eli,

Here it is the offending callstack.

https://gist.github.com/yyoncho/7032464b456f60270100c747f42885f8

Thanks,
Ivan


However, I cannot reproduce what you describe, not with the latest
> Emacs master branch.  When I repeat the recipe you posted in
>
>   https://gist.github.com/yyoncho/9e9c4e14734fdd9a22d6600a88a27ae1
>
> I get zero as the final value of the counter, not 32982.  As I'd
> expect, as I don't see why kill-buffer-query-functions would be called
> in this scenario.  This was in "emacs -Q", do you get a non-zero count
> even in "emacs -Q"?
>

I am also unable to reproduce it with "emacs -q", even it works fine right
after I start my Spacemacs based configuration. It happens after some time
on my system.


>
> To see why kill-buffer-query-functions is invoked in your case, I
> suggest the following:
>
>   . Modify the above recipe to call 'message' inside
>     my/test-query-function.
>   . Run Emacs under GDB after putting a breakpoint in Fmessage (this
>     is the C name of 'message').
>   . Run your recipe.  When the breakpoint in 'message' breaks, display
>     the Lisp backtrace using the "xbacktrace" command (it is defined
>     in src/.gdbinit in the Emacs source tree).
>
> Alternatively (and maybe easier for you), set debug-on-entry to
> trigger when my/test-query-function is called, then run your recipe,
> and look at the backtrace to see why it was called.
>

This does not work since apparently when the breakpoint is hit the


>
> I hope that either of these two ways will allow you to find the code
> which is responsible for calling kill-buffer-query-functions when JSON
> string is being parsed.  Once we understand that, we could see how to
> avoid it.
>
> >  If so, my suggestion would be to run Emacs under 'perf'
> >  and see which parts of json-parse-string take most of the CPU time,
> >  then we'd have solid basis for discussing the significance of the
> >  calls to json_make_string in this scenario.  If you want me to do the
> >  'perf' run (with the caveat that in "emacs -Q" it might run faster
> >  than in your customized Emacs), please give a complete recipe, and I
> >  will do it.
> >
> > Like I said in the first mail - I cannot give you the recipe since I am
> not able to track down what is causing that
> > slowdown and I need help on that. Can you give me a link on how to run
> emacs under "perf"?
>
> Here are a few:
>
>   http://www.brendangregg.com/perf.html
>
> https://perf.wiki.kernel.org/index.php/Tutorial#Sampling_with_perf_record
>
> https://perf.wiki.kernel.org/index.php/Tutorial#Sample_analysis_with_perf_report
>
> The perf man pages are also useful.
>

[-- Attachment #2: Type: text/html, Size: 4081 bytes --]

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Sat, 23 Mar 2019 22:23:49 +0200
> Cc: Sébastien Chapuis <sebastien@chapu.is>, 
> 	31138@debbugs.gnu.org
> 
> Here it is the offending callstack. 
> 
> https://gist.github.com/yyoncho/7032464b456f60270100c747f42885f8

OK, thanks.  I think I see the reason.

Does it work for you to bind run-hooks to nil around calls to
json-parse-string?

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 761 bytes --]

HI Eli,

I will try with run-hooks = nil. (I still cannot reproduce the issue
consistently). As a side note, I think that similar issue affects emacs
dynamic modules based on my limited testing. Please reply back if you need
a callstack.

Thanks,
Ivan


On Sat, Mar 23, 2019 at 10:54 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: yyoncho <yyoncho@gmail.com>
> > Date: Sat, 23 Mar 2019 22:23:49 +0200
> > Cc: Sébastien Chapuis <sebastien@chapu.is>,
> >       31138@debbugs.gnu.org
> >
> > Here it is the offending callstack.
> >
> > https://gist.github.com/yyoncho/7032464b456f60270100c747f42885f8
>
> OK, thanks.  I think I see the reason.
>
> Does it work for you to bind run-hooks to nil around calls to
> json-parse-string?
>

[-- Attachment #2: Type: text/html, Size: 1446 bytes --]

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Sat, 23 Mar 2019 23:04:18 +0200
> Cc: Sébastien Chapuis <sebastien@chapu.is>, 
> 	31138@debbugs.gnu.org
> 
> I will try with run-hooks = nil. (I still cannot reproduce the issue consistently). As a side note, I think that similar
> issue affects emacs dynamic modules based on my limited testing. Please reply back if you need a callstack.

Yes, a callstack in the case of modules would be helpful (although I
think I can guess where it will point).

Thanks.

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 1768 bytes --]

Hi Eli,

The callstack is pretty much the same, here are the relevant parts:

    [-] code_convert_string_norecord@0x000055555565e30d (coding.c:9488)
    [-] module_make_string@0x00005555557d8f0d (emacs-module.c:597)
    [-] emacs::convert::<impl emacs::IntoLisp<’e> for &’a
T>::into_lisp@0x00007fffdeafe8c1 (Unknown source)
    [-] emacs_json_lsp::json_to_lisp@0x00007fffdeb0117e (Unknown source)
    [-] emacs_json_lsp::json_to_lisp@0x00007fffdeb0169f (Unknown source)
    [-] emacs_json_lsp::json_to_lisp@0x00007fffdeb012b3 (Unknown source)
    [-] emacs_json_lsp::json_to_lisp@0x00007fffdeb0169f (Unknown source)
    [-] emacs_json_lsp::json_to_lisp@0x00007fffdeb0169f (Unknown source)
    [-] emacs_json_lsp::parse_string@0x00007fffdeb019a9 (Unknown source)
    [-] std::panicking::try::do_call@0x00007fffdeb02a9f (Unknown source)
    [-] __rust_maybe_catch_panic@0x00007fffdeb2f26a (src/libpanic_unwind/
lib.rs:102)
    [-] <emacs::CallEnv as
emacs::func::HandleCall>::handle_call@0x00007fffdeb0277a (Unknown source)
    [-] emacs_json_lsp::init::extern_lambda@0x00007fffdeb01cc4 (Unknown
source)

Thanks,
Ivan

On Sun, Mar 24, 2019 at 5:32 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: yyoncho <yyoncho@gmail.com>
> > Date: Sat, 23 Mar 2019 23:04:18 +0200
> > Cc: Sébastien Chapuis <sebastien@chapu.is>,
> >       31138@debbugs.gnu.org
> >
> > I will try with run-hooks = nil. (I still cannot reproduce the issue
> consistently). As a side note, I think that similar
> > issue affects emacs dynamic modules based on my limited testing. Please
> reply back if you need a callstack.
>
> Yes, a callstack in the case of modules would be helpful (although I
> think I can guess where it will point).
>
> Thanks.
>

[-- Attachment #2: Type: text/html, Size: 2560 bytes --]

bug#31138: Native json slower than json.el

[Eli Zaretskii]

On March 24, 2019 11:35:25 AM GMT+02:00, yyoncho <yyoncho@gmail.com> wrote:
> Hi Eli,
> 
> The callstack is pretty much the same, here are the relevant parts:
> 
>    [-] code_convert_string_norecord@0x000055555565e30d (coding.c:9488)
>     [-] module_make_string@0x00005555557d8f0d (emacs-module.c:597)
>     [-] emacs::convert::<impl emacs::IntoLisp<’e> for &’a
> T>::into_lisp@0x00007fffdeafe8c1 (Unknown source)
>   [-] emacs_json_lsp::json_to_lisp@0x00007fffdeb0117e (Unknown source)
>   [-] emacs_json_lsp::json_to_lisp@0x00007fffdeb0169f (Unknown source)
>   [-] emacs_json_lsp::json_to_lisp@0x00007fffdeb012b3 (Unknown source)
>   [-] emacs_json_lsp::json_to_lisp@0x00007fffdeb0169f (Unknown source)
>   [-] emacs_json_lsp::json_to_lisp@0x00007fffdeb0169f (Unknown source)
>   [-] emacs_json_lsp::parse_string@0x00007fffdeb019a9 (Unknown source)
>   [-] std::panicking::try::do_call@0x00007fffdeb02a9f (Unknown source)
>  [-] __rust_maybe_catch_panic@0x00007fffdeb2f26a (src/libpanic_unwind/
> lib.rs:102)
>     [-] <emacs::CallEnv as
> emacs::func::HandleCall>::handle_call@0x00007fffdeb0277a (Unknown
> source)
>    [-] emacs_json_lsp::init::extern_lambda@0x00007fffdeb01cc4 (Unknown
> source)
> 
> Thanks,
> Ivan
> 
> On Sun, Mar 24, 2019 at 5:32 AM Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > > From: yyoncho <yyoncho@gmail.com>
> > > Date: Sat, 23 Mar 2019 23:04:18 +0200
> > > Cc: Sébastien Chapuis <sebastien@chapu.is>,
> > >       31138@debbugs.gnu.org
> > >
> > > I will try with run-hooks = nil. (I still cannot reproduce the
> issue
> > consistently). As a side note, I think that similar
> > > issue affects emacs dynamic modules based on my limited testing.
> Please
> > reply back if you need a callstack.
> >
> > Yes, a callstack in the case of modules would be helpful (although I
> > think I can guess where it will point).
> >
> > Thanks.
> >

Thanks, but where is the call to the hook which iscthe source of the problem?  I don't even see Fkill_buffer in the backtrace.  What am I missing?

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 2403 bytes --]

After code_convert_string_norecord the callstack is the same as the
callstack of the JSON parsing (it uses the same method to convert/validate
the string).



On Sun, 24 Mar 2019, 13:20 Eli Zaretskii, <eliz@gnu.org> wrote:

> On March 24, 2019 11:35:25 AM GMT+02:00, yyoncho <yyoncho@gmail.com>
> wrote:
> > Hi Eli,
> >
> > The callstack is pretty much the same, here are the relevant parts:
> >
> >    [-] code_convert_string_norecord@0x000055555565e30d (coding.c:9488)
> >     [-] module_make_string@0x00005555557d8f0d (emacs-module.c:597)
> >     [-] emacs::convert::<impl emacs::IntoLisp<’e> for &’a
> > T>::into_lisp@0x00007fffdeafe8c1 (Unknown source)
> >   [-] emacs_json_lsp::json_to_lisp@0x00007fffdeb0117e (Unknown source)
> >   [-] emacs_json_lsp::json_to_lisp@0x00007fffdeb0169f (Unknown source)
> >   [-] emacs_json_lsp::json_to_lisp@0x00007fffdeb012b3 (Unknown source)
> >   [-] emacs_json_lsp::json_to_lisp@0x00007fffdeb0169f (Unknown source)
> >   [-] emacs_json_lsp::json_to_lisp@0x00007fffdeb0169f (Unknown source)
> >   [-] emacs_json_lsp::parse_string@0x00007fffdeb019a9 (Unknown source)
> >   [-] std::panicking::try::do_call@0x00007fffdeb02a9f (Unknown source)
> >  [-] __rust_maybe_catch_panic@0x00007fffdeb2f26a (src/libpanic_unwind/
> > lib.rs:102)
> >     [-] <emacs::CallEnv as
> > emacs::func::HandleCall>::handle_call@0x00007fffdeb0277a (Unknown
> > source)
> >    [-] emacs_json_lsp::init::extern_lambda@0x00007fffdeb01cc4 (Unknown
> > source)
> >
> > Thanks,
> > Ivan
> >
> > On Sun, Mar 24, 2019 at 5:32 AM Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > > > From: yyoncho <yyoncho@gmail.com>
> > > > Date: Sat, 23 Mar 2019 23:04:18 +0200
> > > > Cc: Sébastien Chapuis <sebastien@chapu.is>,
> > > >       31138@debbugs.gnu.org
> > > >
> > > > I will try with run-hooks = nil. (I still cannot reproduce the
> > issue
> > > consistently). As a side note, I think that similar
> > > > issue affects emacs dynamic modules based on my limited testing.
> > Please
> > > reply back if you need a callstack.
> > >
> > > Yes, a callstack in the case of modules would be helpful (although I
> > > think I can guess where it will point).
> > >
> > > Thanks.
> > >
>
> Thanks, but where is the call to the hook which iscthe source of the
> problem?  I don't even see Fkill_buffer in the backtrace.  What am I
> missing?
>

[-- Attachment #2: Type: text/html, Size: 3648 bytes --]

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Sun, 24 Mar 2019 13:37:23 +0200
> Cc: Sébastien Chapuis <sebastien@chapu.is>, 
> 	31138@debbugs.gnu.org
> 
> After code_convert_string_norecord the callstack is the same as the callstack of the JSON parsing (it uses
> the same method to convert/validate the string).  

Got it, thanks.

Please try the patch below and see if it solves the problem in both
cases.

diff --git a/src/coding.c b/src/coding.c
index 905c7ce..f9c9dc8 100644
--- a/src/coding.c
+++ b/src/coding.c
@@ -7807,10 +7807,14 @@ make_conversion_work_buffer (bool multibyte)
   Lisp_Object name, workbuf;
   struct buffer *current;
 
+  current = current_buffer;
   if (reused_workbuf_in_use)
     {
       name = Fgenerate_new_buffer_name (Vcode_conversion_workbuf_name, Qnil);
       workbuf = Fget_buffer_create (name);
+      set_buffer_internal (XBUFFER (workbuf));
+      Fset (Fmake_local_variable (Qkill_buffer_query_functions), Qnil);
+      Fset (Fmake_local_variable (Qkill_buffer_hook), Qnil);
     }
   else
     {
@@ -7819,9 +7823,8 @@ make_conversion_work_buffer (bool multibyte)
 	Vcode_conversion_reused_workbuf
 	  = Fget_buffer_create (Vcode_conversion_workbuf_name);
       workbuf = Vcode_conversion_reused_workbuf;
+      set_buffer_internal (XBUFFER (workbuf));
     }
-  current = current_buffer;
-  set_buffer_internal (XBUFFER (workbuf));
   /* We can't allow modification hooks to run in the work buffer.  For
      instance, directory_files_internal assumes that file decoding
      doesn't compile new regexps.  */

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 1952 bytes --]

Hi Eli,

Before starting to test the patch I saw that Fget_buffer_create is
running Qbuffer_list_update_hook - do we need to do a similar thing for it
too?

Thanks,
Ivan

On Sun, Mar 24, 2019 at 5:15 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: yyoncho <yyoncho@gmail.com>
> > Date: Sun, 24 Mar 2019 13:37:23 +0200
> > Cc: Sébastien Chapuis <sebastien@chapu.is>,
> >       31138@debbugs.gnu.org
> >
> > After code_convert_string_norecord the callstack is the same as the
> callstack of the JSON parsing (it uses
> > the same method to convert/validate the string).
>
> Got it, thanks.
>
> Please try the patch below and see if it solves the problem in both
> cases.
>
> diff --git a/src/coding.c b/src/coding.c
> index 905c7ce..f9c9dc8 100644
> --- a/src/coding.c
> +++ b/src/coding.c
> @@ -7807,10 +7807,14 @@ make_conversion_work_buffer (bool multibyte)
>    Lisp_Object name, workbuf;
>    struct buffer *current;
>
> +  current = current_buffer;
>    if (reused_workbuf_in_use)
>      {
>        name = Fgenerate_new_buffer_name (Vcode_conversion_workbuf_name,
> Qnil);
>        workbuf = Fget_buffer_create (name);
> +      set_buffer_internal (XBUFFER (workbuf));
> +      Fset (Fmake_local_variable (Qkill_buffer_query_functions), Qnil);
> +      Fset (Fmake_local_variable (Qkill_buffer_hook), Qnil);
>      }
>    else
>      {
> @@ -7819,9 +7823,8 @@ make_conversion_work_buffer (bool multibyte)
>         Vcode_conversion_reused_workbuf
>           = Fget_buffer_create (Vcode_conversion_workbuf_name);
>        workbuf = Vcode_conversion_reused_workbuf;
> +      set_buffer_internal (XBUFFER (workbuf));
>      }
> -  current = current_buffer;
> -  set_buffer_internal (XBUFFER (workbuf));
>    /* We can't allow modification hooks to run in the work buffer.  For
>       instance, directory_files_internal assumes that file decoding
>       doesn't compile new regexps.  */
>

[-- Attachment #2: Type: text/html, Size: 2702 bytes --]

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Sun, 24 Mar 2019 20:24:35 +0200
> Cc: Sébastien Chapuis <sebastien@chapu.is>, 
> 	31138@debbugs.gnu.org
> 
> Before starting to test the patch I saw that Fget_buffer_create is running Qbuffer_list_update_hook - do we
> need to do a similar thing for it too?

Maybe, but let's first see if the patch as I sent is already producing
some positive effect, shall we?

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 3070 bytes --]

Hi Eli,

From what I can see the Qbuffer_list_update_hook is called.

- command-execute                                                 865  92%
 - call-interactively                                             865  92%
  - funcall-interactively                                         865  92%
   - eval-defun                                                   865  92%
    - apply                                                       865  92%
     - edebug-eval-defun                                          865  92%
      - eval                                                      865  92%
       - progn                                                    865  92%
        - message                                                 865  92%
         - benchmark-run                                          865  92%
          - let                                                   865  92%
           - list                                                 865  92%
            - benchmark-elapse                                    865  92%
             - let                                                865  92%
              - ignore                                            865  92%
               - json-parse-string                                596  63%
                - run-hooks                                       303  32%
                 - doom-modeline-update-persp-name                116  12%
                  - get-current-persp                             110  11%
                     get-frame-persp                               96  10%
                   + window-persp-set-p                             4   0%
                     #<compiled 0x1594da86eb25>                     1   0%
                 + doom-modeline-set-selected-window                  7   0%
                - #<compiled 0x1594da2b4a39>                      268  28%
                 + spacemacs/useful-buffer-p                      245  26%
                 + #<lambda 0x85a9e12ee490302>                     17   1%
                + replace-buffer-in-windows                        21   2%
+ ...                                                              74   7%

I want to share some observations:

1. The hooks are not called initially, I am not sure what triggers them.
This makes the json parsing extremely slow(100 times)
2. Even without the hooks, the JSON parsing is slower than the emacs -q
native parsing when I am running my setup. This will require additional
investigation.

Thanks,
Ivan

On Sun, Mar 24, 2019 at 8:28 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: yyoncho <yyoncho@gmail.com>
> > Date: Sun, 24 Mar 2019 20:24:35 +0200
> > Cc: Sébastien Chapuis <sebastien@chapu.is>,
> >       31138@debbugs.gnu.org
> >
> > Before starting to test the patch I saw that Fget_buffer_create is
> running Qbuffer_list_update_hook - do we
> > need to do a similar thing for it too?
>
> Maybe, but let's first see if the patch as I sent is already producing
> some positive effect, shall we?
>

[-- Attachment #2: Type: text/html, Size: 4746 bytes --]

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Sun, 24 Mar 2019 22:57:58 +0200
> Cc: Sébastien Chapuis <sebastien@chapu.is>, 
> 	31138@debbugs.gnu.org
> 
> From what I can see the Qbuffer_list_update_hook is called.

Is this with or without the patch I sent?

> 1. The hooks are not called initially, I am not sure what triggers them. This makes the json parsing extremely
> slow(100 times)

The hooks are not called when decoding strings uses a fixed working
buffer that is created once and never killed.  Then something in your
setup causes this single working buffer to become in-use, and Emacs
then allocates an additional buffer, which is created and killed each
time we need to decode a string.  Both creating and killing of that
buffer runs the buffer-list-update-hook.

> 2. Even without the hooks, the JSON parsing is slower than the emacs -q native parsing when I am running
> my setup. This will require additional investigation. 

Please, let's first finish fixing the factors we do understand already
and see what effect that produces.  Otherwise I don't see how we can
make steady progress in this matter.  That is why I sent an initial,
most probably incomplete patch.  Please apply it and see what it fixes
and what it doesn't fix.

I must rely on you in this matter, because I cannot reproduce the
slowdown on my system.

Thanks.

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 1909 bytes --]

Hi Eli,


> Is this with or without the patch I sent?
>

It is with the patch (in the two profiler.el reports, the "kill" hooks are
not present)


>
> > 1. The hooks are not called initially, I am not sure what triggers them.
> This makes the json parsing extremely
> > slow(100 times)
>
> The hooks are not called when decoding strings uses a fixed working
> buffer that is created once and never killed.  Then something in your
> setup causes this single working buffer to become in-use, and Emacs
> then allocates an additional buffer, which is created and killed each
> time we need to decode a string.  Both creating and killing of that
> buffer runs the buffer-list-update-hook.
>

Just found that it is not my setup which triggers the issue bug pressing
C-g during the parsing:

To reproduce:

(benchmark-run 10 (json-parse-string json-string))
(benchmark-run 10000000 (json-parse-string json-string))
(benchmark-run 10 (json-parse-string json-string))

Press C-g when second benchmark-run is processed.


> > 2. Even without the hooks, the JSON parsing is slower than the emacs -q
> native parsing when I am running
> > my setup. This will require additional investigation.
>
> Please, let's first finish fixing the factors we do understand already
> and see what effect that produces.  Otherwise I don't see how we can
> make steady progress in this matter.  That is why I sent an initial,
> most probably incomplete patch.  Please apply it and see what it fixes
> and what it doesn't fix.
>


I did exactly what you are saying here: I applied the patch and listed the
effects:
1. The buffer-list-update-hook is called
2. The patch does not affect the perf difference between emacs -q and my
setup.

> I must rely on you in this matter, because I cannot reproduce the
> slowdown on my system.

I realize that and I am trying to be as helpful as possible by sharing all
my observations.

Thanks,
Ivan

[-- Attachment #2: Type: text/html, Size: 2873 bytes --]

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Mon, 25 Mar 2019 07:44:00 +0200
> Cc: Sébastien Chapuis <sebastien@chapu.is>, 
> 	31138@debbugs.gnu.org
> 
>  Is this with or without the patch I sent?
> 
> It is with the patch (in the two profiler.el reports, the "kill" hooks are not present)

OK, thanks.

> Just found that it is not my setup which triggers the issue bug pressing C-g during the parsing:

Right, I think I see the reason for that.  The patch below should fix
this.  It also makes buffer-list-update-hook nil when creating and
killing the conversion buffer, for a good measure.  Please note that
the patch is relative to the current master, so be sure to run
"git checkout src/coding.c" before applying.

> I did exactly what you are saying here: I applied the patch and listed the effects:
> 1. The buffer-list-update-hook is called
> 2. The patch does not affect the perf difference between emacs -q and my setup.
> 
> > I must rely on you in this matter, because I cannot reproduce the
> > slowdown on my system.
> 
> I realize that and I am trying to be as helpful as possible by sharing all my observations.

OK, but please always say if your observations are before or after the
last patch I sent.

Thanks.

Here's the patch:

diff --git a/src/coding.c b/src/coding.c
index 905c7ce..c6d9643 100644
--- a/src/coding.c
+++ b/src/coding.c
@@ -7798,42 +7798,6 @@ static Lisp_Object Vcode_conversion_reused_workbuf;
 static bool reused_workbuf_in_use;
 
 
-/* Return a working buffer of code conversion.  MULTIBYTE specifies the
-   multibyteness of returning buffer.  */
-
-static Lisp_Object
-make_conversion_work_buffer (bool multibyte)
-{
-  Lisp_Object name, workbuf;
-  struct buffer *current;
-
-  if (reused_workbuf_in_use)
-    {
-      name = Fgenerate_new_buffer_name (Vcode_conversion_workbuf_name, Qnil);
-      workbuf = Fget_buffer_create (name);
-    }
-  else
-    {
-      reused_workbuf_in_use = 1;
-      if (NILP (Fbuffer_live_p (Vcode_conversion_reused_workbuf)))
-	Vcode_conversion_reused_workbuf
-	  = Fget_buffer_create (Vcode_conversion_workbuf_name);
-      workbuf = Vcode_conversion_reused_workbuf;
-    }
-  current = current_buffer;
-  set_buffer_internal (XBUFFER (workbuf));
-  /* We can't allow modification hooks to run in the work buffer.  For
-     instance, directory_files_internal assumes that file decoding
-     doesn't compile new regexps.  */
-  Fset (Fmake_local_variable (Qinhibit_modification_hooks), Qt);
-  Ferase_buffer ();
-  bset_undo_list (current_buffer, Qt);
-  bset_enable_multibyte_characters (current_buffer, multibyte ? Qt : Qnil);
-  set_buffer_internal (current);
-  return workbuf;
-}
-
-
 static void
 code_conversion_restore (Lisp_Object arg)
 {
@@ -7846,7 +7810,12 @@ code_conversion_restore (Lisp_Object arg)
       if (EQ (workbuf, Vcode_conversion_reused_workbuf))
 	reused_workbuf_in_use = 0;
       else
-	Fkill_buffer (workbuf);
+	{
+	  ptrdiff_t count = SPECPDL_INDEX ();
+	  specbind (Qbuffer_list_update_hook, Qnil);
+	  Fkill_buffer (workbuf);
+	  unbind_to (count, Qnil);
+	}
     }
   set_buffer_internal (XBUFFER (current));
 }
@@ -7857,9 +7826,51 @@ code_conversion_save (bool with_work_buf, bool multibyte)
   Lisp_Object workbuf = Qnil;
 
   if (with_work_buf)
-    workbuf = make_conversion_work_buffer (multibyte);
+    {
+      ptrdiff_t count = SPECPDL_INDEX ();
+      if (reused_workbuf_in_use)
+	{
+	  Lisp_Object name
+	    = Fgenerate_new_buffer_name (Vcode_conversion_workbuf_name, Qnil);
+	  specbind (Qbuffer_list_update_hook, Qnil);
+	  workbuf = Fget_buffer_create (name);
+	  unbind_to (count, Qnil);
+	}
+      else
+	{
+	  if (NILP (Fbuffer_live_p (Vcode_conversion_reused_workbuf)))
+	    {
+	      specbind (Qbuffer_list_update_hook, Qnil);
+	      Vcode_conversion_reused_workbuf
+		= Fget_buffer_create (Vcode_conversion_workbuf_name);
+	      unbind_to (count, Qnil);
+	    }
+	  workbuf = Vcode_conversion_reused_workbuf;
+	}
+    }
   record_unwind_protect (code_conversion_restore,
 			 Fcons (Fcurrent_buffer (), workbuf));
+  if (!NILP (workbuf))
+    {
+      struct buffer *current = current_buffer;
+      set_buffer_internal (XBUFFER (workbuf));
+      /* We can't allow modification hooks to run in the work buffer.  For
+	 instance, directory_files_internal assumes that file decoding
+	 doesn't compile new regexps.  */
+      Fset (Fmake_local_variable (Qinhibit_modification_hooks), Qt);
+      Ferase_buffer ();
+      bset_undo_list (current_buffer, Qt);
+      bset_enable_multibyte_characters (current_buffer, multibyte ? Qt : Qnil);
+      if (EQ (workbuf, Vcode_conversion_reused_workbuf))
+	reused_workbuf_in_use = 1;
+      else
+	{
+	  Fset (Fmake_local_variable (Qkill_buffer_query_functions), Qnil);
+	  Fset (Fmake_local_variable (Qkill_buffer_hook), Qnil);
+	}
+      set_buffer_internal (current);
+    }
+
   return workbuf;
 }
 

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 5732 bytes --]

Hi Eli,

The patch seems to be addressing the hooks related issue.

Thanks,
Ivan

On Mon, Mar 25, 2019 at 6:42 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: yyoncho <yyoncho@gmail.com>
> > Date: Mon, 25 Mar 2019 07:44:00 +0200
> > Cc: Sébastien Chapuis <sebastien@chapu.is>,
> >       31138@debbugs.gnu.org
> >
> >  Is this with or without the patch I sent?
> >
> > It is with the patch (in the two profiler.el reports, the "kill" hooks
> are not present)
>
> OK, thanks.
>
> > Just found that it is not my setup which triggers the issue bug pressing
> C-g during the parsing:
>
> Right, I think I see the reason for that.  The patch below should fix
> this.  It also makes buffer-list-update-hook nil when creating and
> killing the conversion buffer, for a good measure.  Please note that
> the patch is relative to the current master, so be sure to run
> "git checkout src/coding.c" before applying.
>
> > I did exactly what you are saying here: I applied the patch and listed
> the effects:
> > 1. The buffer-list-update-hook is called
> > 2. The patch does not affect the perf difference between emacs -q and my
> setup.
> >
> > > I must rely on you in this matter, because I cannot reproduce the
> > > slowdown on my system.
> >
> > I realize that and I am trying to be as helpful as possible by sharing
> all my observations.
>
> OK, but please always say if your observations are before or after the
> last patch I sent.
>
> Thanks.
>
> Here's the patch:
>
> diff --git a/src/coding.c b/src/coding.c
> index 905c7ce..c6d9643 100644
> --- a/src/coding.c
> +++ b/src/coding.c
> @@ -7798,42 +7798,6 @@ static Lisp_Object Vcode_conversion_reused_workbuf;
>  static bool reused_workbuf_in_use;
>
>
> -/* Return a working buffer of code conversion.  MULTIBYTE specifies the
> -   multibyteness of returning buffer.  */
> -
> -static Lisp_Object
> -make_conversion_work_buffer (bool multibyte)
> -{
> -  Lisp_Object name, workbuf;
> -  struct buffer *current;
> -
> -  if (reused_workbuf_in_use)
> -    {
> -      name = Fgenerate_new_buffer_name (Vcode_conversion_workbuf_name,
> Qnil);
> -      workbuf = Fget_buffer_create (name);
> -    }
> -  else
> -    {
> -      reused_workbuf_in_use = 1;
> -      if (NILP (Fbuffer_live_p (Vcode_conversion_reused_workbuf)))
> -       Vcode_conversion_reused_workbuf
> -         = Fget_buffer_create (Vcode_conversion_workbuf_name);
> -      workbuf = Vcode_conversion_reused_workbuf;
> -    }
> -  current = current_buffer;
> -  set_buffer_internal (XBUFFER (workbuf));
> -  /* We can't allow modification hooks to run in the work buffer.  For
> -     instance, directory_files_internal assumes that file decoding
> -     doesn't compile new regexps.  */
> -  Fset (Fmake_local_variable (Qinhibit_modification_hooks), Qt);
> -  Ferase_buffer ();
> -  bset_undo_list (current_buffer, Qt);
> -  bset_enable_multibyte_characters (current_buffer, multibyte ? Qt :
> Qnil);
> -  set_buffer_internal (current);
> -  return workbuf;
> -}
> -
> -
>  static void
>  code_conversion_restore (Lisp_Object arg)
>  {
> @@ -7846,7 +7810,12 @@ code_conversion_restore (Lisp_Object arg)
>        if (EQ (workbuf, Vcode_conversion_reused_workbuf))
>         reused_workbuf_in_use = 0;
>        else
> -       Fkill_buffer (workbuf);
> +       {
> +         ptrdiff_t count = SPECPDL_INDEX ();
> +         specbind (Qbuffer_list_update_hook, Qnil);
> +         Fkill_buffer (workbuf);
> +         unbind_to (count, Qnil);
> +       }
>      }
>    set_buffer_internal (XBUFFER (current));
>  }
> @@ -7857,9 +7826,51 @@ code_conversion_save (bool with_work_buf, bool
> multibyte)
>    Lisp_Object workbuf = Qnil;
>
>    if (with_work_buf)
> -    workbuf = make_conversion_work_buffer (multibyte);
> +    {
> +      ptrdiff_t count = SPECPDL_INDEX ();
> +      if (reused_workbuf_in_use)
> +       {
> +         Lisp_Object name
> +           = Fgenerate_new_buffer_name (Vcode_conversion_workbuf_name,
> Qnil);
> +         specbind (Qbuffer_list_update_hook, Qnil);
> +         workbuf = Fget_buffer_create (name);
> +         unbind_to (count, Qnil);
> +       }
> +      else
> +       {
> +         if (NILP (Fbuffer_live_p (Vcode_conversion_reused_workbuf)))
> +           {
> +             specbind (Qbuffer_list_update_hook, Qnil);
> +             Vcode_conversion_reused_workbuf
> +               = Fget_buffer_create (Vcode_conversion_workbuf_name);
> +             unbind_to (count, Qnil);
> +           }
> +         workbuf = Vcode_conversion_reused_workbuf;
> +       }
> +    }
>    record_unwind_protect (code_conversion_restore,
>                          Fcons (Fcurrent_buffer (), workbuf));
> +  if (!NILP (workbuf))
> +    {
> +      struct buffer *current = current_buffer;
> +      set_buffer_internal (XBUFFER (workbuf));
> +      /* We can't allow modification hooks to run in the work buffer.  For
> +        instance, directory_files_internal assumes that file decoding
> +        doesn't compile new regexps.  */
> +      Fset (Fmake_local_variable (Qinhibit_modification_hooks), Qt);
> +      Ferase_buffer ();
> +      bset_undo_list (current_buffer, Qt);
> +      bset_enable_multibyte_characters (current_buffer, multibyte ? Qt :
> Qnil);
> +      if (EQ (workbuf, Vcode_conversion_reused_workbuf))
> +       reused_workbuf_in_use = 1;
> +      else
> +       {
> +         Fset (Fmake_local_variable (Qkill_buffer_query_functions), Qnil);
> +         Fset (Fmake_local_variable (Qkill_buffer_hook), Qnil);
> +       }
> +      set_buffer_internal (current);
> +    }
> +
>    return workbuf;
>  }
>
>

[-- Attachment #2: Type: text/html, Size: 6932 bytes --]

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Mon, 25 Mar 2019 20:20:57 +0200
> Cc: Sébastien Chapuis <sebastien@chapu.is>, 
> 	31138@debbugs.gnu.org
> 
> The patch seems to be addressing the hooks related issue. 

Yes.  Do you see improvement in performance?

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 862 bytes --]

Hi Eli,

I am unable to see any difference in the performance of the json parsing in
emacs -q and in my setup - it is still ~2 times slower. I believe that it
is caused by code_convert_string . I compiled emacs without that call and
there is no difference in performance in both setups and the parsing is 2
times faster than emacs -q with code_convert_string.

I want to discuss the native json performance in the context of lsp-mode
needs. Is it ok to do it in this thread?

Thanks,
Ivan

On Mon, Mar 25, 2019 at 8:25 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: yyoncho <yyoncho@gmail.com>
> > Date: Mon, 25 Mar 2019 20:20:57 +0200
> > Cc: Sébastien Chapuis <sebastien@chapu.is>,
> >       31138@debbugs.gnu.org
> >
> > The patch seems to be addressing the hooks related issue.
>
> Yes.  Do you see improvement in performance?
>

[-- Attachment #2: Type: text/html, Size: 1463 bytes --]

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Mon, 25 Mar 2019 21:16:53 +0200
> Cc: Sébastien Chapuis <sebastien@chapu.is>, 
> 	31138@debbugs.gnu.org
> 
> I am unable to see any difference in the performance of the json parsing in emacs -q and in my setup - it is
> still ~2 times slower.

You mean, in your setup it's twice slower than in "emacs -Q"?

And you are saying that the changes I made have no effect on the
performance?  Then what about the 100-fold slowdown you were talking
about, allegedly caused by the hooks?

> I believe that it is caused by code_convert_string .

This needs some more specific explanation, because code_convert_string
is called in both your setup and in "emacs -Q".  So it isn't
code_convert_string itself, it's something else, perhaps triggered by
code_convert_string, like those hooks I disabled.

> I compiled emacs without that call and
> there is no difference in performance in both setups and the parsing is 2 times faster than emacs -q with
> code_convert_string.

It's small wonder that removing code makes functions which called that
code work faster.  What does removing code_convert_string achieve
except showing this truism in action?

> I want to discuss the native json performance in the context of lsp-mode needs. Is it ok to do it in this thread?

It depends on what you want to discuss, exactly.

And I'm still confused regarding the performance that bothers you.  Is
the problem the two-fold slowdown relative to "emacs -Q", or is the
problem much worse slowdown in some other situation?  Is the patch I
sent useful and should be pushed, or do you no longer care about it
because it doesn't help you?

I feel that I no longer understand what problem we are trying to
solve, and that no matter what improvements I propose, the discussion
always ends up insisting that code_convert_string is the culprit.

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 3724 bytes --]

Hi Eli,

You mean, in your setup it's twice slower than in "emacs -Q"?
>

Yes.


> And you are saying that the changes I made have no effect on the
> performance?  Then what about the 100-fold slowdown you were talking
> about, allegedly caused by the hooks?
>

The slowdown caused by the hooks was caused by the C-g and the issue does
not exist and it is fixed by your patch. Otherwise, it is ~0.6 vs ~1.2secs.


> > I believe that it is caused by code_convert_string .
>
> This needs some more specific explanation, because code_convert_string
> is called in both your setup and in "emacs -Q".  So it isn't
> code_convert_string itself, it's something else, perhaps triggered by
> code_convert_string, like those hooks I disabled.
>

Yes, I think that it is related to switching to different buffers.


>
> > I compiled emacs without that call and
> > there is no difference in performance in both setups and the parsing is
> 2 times faster than emacs -q with
> > code_convert_string.
>
> It's small wonder that removing code makes functions which called that
> code work faster.  What does removing code_convert_string achieve
> except showing this truism in action?
>

It is actually very helpful for me and I guess for you:

1. Now we know where to look for if we want to optimize further emacs -q
parsing performance.
2. Now we know what the difference between emacs -q performance and my
setup is caused by something in that function.


>
> > I want to discuss the native json performance in the context of lsp-mode
> needs. Is it ok to do it in this thread?
>
> It depends on what you want to discuss, exactly.
>
> And I'm still confused regarding the performance that bothers you.  Is
> the problem the two-fold slowdown relative to "emacs -Q", or is the
> problem much worse slowdown in some other situation?  Is the patch I
> sent useful and should be pushed, or do you no longer care about it
> because it doesn't help you?
>

IMO the fix should go in, I summarized what are lsp-mode problems.


> I feel that I no longer understand what problem we are trying to
> solve, and that no matter what improvements I propose, the discussion
> always ends up insisting that code_convert_string is the culprit.
>

...and it actually *was* the culprit, right?  Furthermore, it the
performance test that I mentioned proves it is *still* the culprit...

Here it is the summary of the issues that I see in native json parsing from
lsp-mode (beware that I will list issues that are not relevant to that bug
bug#31138).

1. Hooks might be triggered when performing json parsing(fixed by you). I
hoped that this will fix 2) but apparently these are two separate issues.
2. There is a difference in performance between emacs -q and non-"emacs -q"
. I would expect json parsing to be side effect free. (My guess is that it
is related to switching buffers because with-temp-buffer has some positive
effect).
3. Even in the best case scenario native parsing is not fast enough. In
lsp-mode, you might receive 3.5mb json as a server-side response while you
are typing. E. g. I might type "abc" and get 10.5 mb json. I compared the
performance of nodejs parsing and it is ~10 times faster than emacs -q.
4. JSON parsing can be performed only on the UI thread.

Alternatively, we (lsp-mode team) will be able to solve all these in a
dynamic module if emacs module mechanism is extended to allow creating
emacs lisp structures efficiently.

PS: I really enjoy the ironical passive-aggressive style of communication
which I guess is common for emacs-devel but I will still be trying to be
constructive. I just want you to know that I won't bother answering and
focus only on the stuff that will help to solve the issue...

Thanks,
Ivan

[-- Attachment #2: Type: text/html, Size: 5209 bytes --]

bug#31138: Native json slower than json.el

[Dmitry Gutov]

Hi Ivan,

On 25.03.2019 23:34, yyoncho wrote:

> PS: I really enjoy the ironical passive-aggressive style of 
> communication which I guess is common for emacs-devel but I will still 
> be trying to be constructive. I just want you to know that I won't 
> bother answering and focus only on the stuff that will help to solve the 
> issue...

While the recent message clarified quite a bit, the preceding ones were 
not so easy to interpret, and to understand that you applied the 
necessary rigor when benchmarking, and which results you got.

I'm not sure where you see the "ironical" style of conversation, but 
maybe some questions seemed rhetorical to you. I'm pretty sure they weren't.

P.S. Way to go to label the whole mailing list as "passive-aggressive" 
while calling yourself "constructive".

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Mon, 25 Mar 2019 23:34:38 +0200
> Cc: Sébastien Chapuis <sebastien@chapu.is>, 
> 	31138@debbugs.gnu.org
> 
>  You mean, in your setup it's twice slower than in "emacs -Q"?
>  
> Yes.

Then my suggestion to bisect your setup still stands.  Alternatively,
if you can find a recipe where this happens and post it, I will look
into it.

>  And you are saying that the changes I made have no effect on the
>  performance?  Then what about the 100-fold slowdown you were talking
>  about, allegedly caused by the hooks?
> 
> The slowdown caused by the hooks was caused by the C-g and the issue does not exist and it is fixed by
> your patch. Otherwise, it is ~0.6 vs ~1.2secs. 

For 10 benchmark runs?

And even if it's twice slower than "emacs -Q", it should be still
almost twice faster than the Lisp implementation, at least that's what
I see on 2 different machines with 2 different OSes.

>  > I believe that it is caused by code_convert_string .
> 
>  This needs some more specific explanation, because code_convert_string
>  is called in both your setup and in "emacs -Q".  So it isn't
>  code_convert_string itself, it's something else, perhaps triggered by
>  code_convert_string, like those hooks I disabled.
> 
> Yes, I think that it is related to switching to different buffers.

Then json-parse-buffer doesn't have this problem in your setup?

> 1. Now we know where to look for if we want to optimize further emacs -q parsing performance.

I'm not sure we know that.  We didn't analyze the profile below the
Lisp primitives level.  If we want to see what takes significant parts
of the time in the benchmark runs, we need to use perf and look at the
C level below the primitives

> 2. Now we know what the difference between emacs -q performance and my setup is caused by something in
> that function.

No, I don't think we know that.  You say above that it's caused by
switching buffers, which is not in code_convert_string.

> IMO the fix should go in

OK, will do.

> I summarized what are lsp-mode problems. 

What are they?  Can you summarize them again, please, or point me to
where you did so?

>  I feel that I no longer understand what problem we are trying to
>  solve, and that no matter what improvements I propose, the discussion
>  always ends up insisting that code_convert_string is the culprit.
> 
> ...and it actually *was* the culprit, right?

No, the culprit was the hooks we called as a side effect of that function.

> Furthermore, it the performance test that I mentioned proves it is
> *still* the culprit...

I'm sorry, but I don't see that proof.  Can you explain how you arrive
at that conclusion?

> Here it is the summary of the issues that I see in native json parsing from lsp-mode (beware that I will list
> issues that are not relevant to that bug bug#31138).
> 
> 1. Hooks might be triggered when performing json parsing(fixed by you). I hoped that this will fix 2) but
> apparently these are two separate issues. 

Agreed.  So the issue with hooks no longer exists (once the patch I
sent is committed).

> 2. There is a difference in performance between emacs -q and non-"emacs -q" . I would expect json parsing
> to be side effect free. (My guess is that it is related to switching buffers because with-temp-buffer has some
> positive effect).

I don't think I understand this.  What side effects are you talking
about?  Switching buffers is not part of json-parse-string.

> 3. Even in the best case scenario native parsing is not fast enough. In lsp-mode, you might receive 3.5mb
> json as a server-side response while you are typing. E. g. I might type "abc" and get 10.5 mb json. I compared
> the performance of nodejs parsing and it is ~10 times faster than emacs -q. 

10 times faster doesn't mean Emacs isn't "fast enough".  The current
native parsing is faster than what we had before in Lisp, so this is
progress.

And if we want to see how to optimize JSON parsing in Emacs even more,
we need to profile the code with a large enough JSON, and see what
takes the time.

> 4. JSON parsing can be performed only on the UI thread. 

You can do that in a separate thread in a module.

> Alternatively, we (lsp-mode team) will be able to solve all these in a dynamic module if emacs module
> mechanism is extended to allow creating emacs lisp structures efficiently. 

What is lacking in the modules mechanism?

> PS: I really enjoy the ironical passive-aggressive style of communication which I guess is common for
> emacs-devel but I will still be trying to be constructive. I just want you to know that I won't bother answering
> and focus only on the stuff that will help to solve the issue...

I actually think that discussions on emacs-devel, with a few
exceptions, are generally to the point and quite productive.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Mon, 25 Mar 2019 23:34:38 +0200
> Cc: Sébastien Chapuis <sebastien@chapu.is>, 
> 	31138@debbugs.gnu.org
> 
> 3. Even in the best case scenario native parsing is not fast enough. In lsp-mode, you might receive 3.5mb
> json as a server-side response while you are typing. E. g. I might type "abc" and get 10.5 mb json.

What kind of throughput do you get with native JSON parsing when you
submit a 10MB JSON to it?  My testing indicates that decoding strings
by UTF-8 runs at about 50 msec per MByte for short strings, and about
4 times faster for very large strings.  How does this compare with the
overall time for processing a 10MB JSON?

> Alternatively, we (lsp-mode team) will be able to solve all these in a dynamic module if emacs module
> mechanism is extended to allow creating emacs lisp structures efficiently. 

I think we should only consider this after we profile JSON parsing and
see which parts take the most processing time.

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 1286 bytes --]

Hi Eli,

I think I get similar results.

* ~5mb/s for ~500b message
* ~15mb/s for ~850kb message

So it is like ~500ms for 10mb.

Thanks,
Ivan


On Tue, Mar 26, 2019 at 6:14 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: yyoncho <yyoncho@gmail.com>
> > Date: Mon, 25 Mar 2019 23:34:38 +0200
> > Cc: Sébastien Chapuis <sebastien@chapu.is>,
> >       31138@debbugs.gnu.org
> >
> > 3. Even in the best case scenario native parsing is not fast enough. In
> lsp-mode, you might receive 3.5mb
> > json as a server-side response while you are typing. E. g. I might type
> "abc" and get 10.5 mb json.
>
> What kind of throughput do you get with native JSON parsing when you
> submit a 10MB JSON to it?  My testing indicates that decoding strings
> by UTF-8 runs at about 50 msec per MByte for short strings, and about
> 4 times faster for very large strings.  How does this compare with the
> overall time for processing a 10MB JSON?
>
> > Alternatively, we (lsp-mode team) will be able to solve all these in a
> dynamic module if emacs module
> > mechanism is extended to allow creating emacs lisp structures
> efficiently.
>
> I think we should only consider this after we profile JSON parsing and
> see which parts take the most processing time.
>

[-- Attachment #2: Type: text/html, Size: 1951 bytes --]

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Tue, 26 Mar 2019 19:45:38 +0200
> Cc: Sébastien Chapuis <sebastien@chapu.is>, 
> 	31138@debbugs.gnu.org
> 
> I think I get similar results.
> 
> * ~5mb/s for ~500b message
> * ~15mb/s for ~850kb message
> 
> So it is like ~500ms for 10mb.

But that only makes sense if close to 100% of JSON is strings.  Is
that really so?  Can I see an example of such a JSON?

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 686 bytes --]

Here it is
https://gist.githubusercontent.com/yyoncho/dec968b69185305ed02741e18b27a82d/raw/334b0a51bc52cc3c98edb8ff4bccb5fc4531842b/large.json

Thanks,
Ivan

On Tue, Mar 26, 2019 at 8:11 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: yyoncho <yyoncho@gmail.com>
> > Date: Tue, 26 Mar 2019 19:45:38 +0200
> > Cc: Sébastien Chapuis <sebastien@chapu.is>,
> >       31138@debbugs.gnu.org
> >
> > I think I get similar results.
> >
> > * ~5mb/s for ~500b message
> > * ~15mb/s for ~850kb message
> >
> > So it is like ~500ms for 10mb.
>
> But that only makes sense if close to 100% of JSON is strings.  Is
> that really so?  Can I see an example of such a JSON?
>

[-- Attachment #2: Type: text/html, Size: 1417 bytes --]

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Mon, 25 Mar 2019 20:20:57 +0200
> Cc: Sébastien Chapuis <sebastien@chapu.is>, 
> 	31138@debbugs.gnu.org
> 
> The patch seems to be addressing the hooks related issue. 

OK, I've now pushed the change, with a different way of avoiding the
calls to the hooks.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

Hi Eli,

On 26.03.2019 18:14, Eli Zaretskii wrote:
> I think we should only consider this after we profile JSON parsing and
> see which parts take the most processing time.

What is our next step here?

I have tried out the comparison (with and without code_convert_string) 
on my own JSON data, and the results are similar: 6MB JSON is parsed in 
~0.48s by default, and in ~0.21s with that call removed.

The JSON parsing is not the biggest bottleneck in my particular case, 
but it would be nice to speed it up anyway.

So what should we profile? The internals of code_convert_string?

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: sebastien@chapu.is, 31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 16 Apr 2019 04:36:43 +0300
> 
> On 26.03.2019 18:14, Eli Zaretskii wrote:
> > I think we should only consider this after we profile JSON parsing and
> > see which parts take the most processing time.
> 
> What is our next step here?
> 
> I have tried out the comparison (with and without code_convert_string) 
> on my own JSON data, and the results are similar: 6MB JSON is parsed in 
> ~0.48s by default, and in ~0.21s with that call removed.
> 
> The JSON parsing is not the biggest bottleneck in my particular case, 
> but it would be nice to speed it up anyway.
> 
> So what should we profile? The internals of code_convert_string?

The entire run of parsing a large JSON.  code_convert_string will be
part of the profile.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 487 bytes --]

On 16.04.2019 5:38, Eli Zaretskii wrote:

>> So what should we profile? The internals of code_convert_string?
> 
> The entire run of parsing a large JSON.  code_convert_string will be
> part of the profile.

All right, my test script and results are attached. Here's how I run it:

sudo perf record -F 99 src/emacs -Q --batch -l ~/examples/elisp/json-test.el
sudo perf report -n --stdio > perf-report.txt

If you have certain recording or output format preferences, please let 
me know.

[-- Attachment #2: json-test.el --]
[-- Type: text/x-emacs-lisp, Size: 175 bytes --]

(defun test ()
  (with-temp-buffer
    (insert-file-contents "~/examples/sample-response.json")
    (goto-char (point-min))
    (json-parse-buffer)))

(benchmark 10 '(test))


[-- Attachment #3: perf-report.txt --]
[-- Type: text/plain, Size: 11068 bytes --]

# To display the perf.data header info, please use --header/--header-only options.
#
#
# Total Lost Samples: 0
#
# Samples: 443  of event 'cycles:ppp'
# Event count (approx.): 13387864690
#
# Overhead       Samples  Command  Shared Object         Symbol                          
# ........  ............  .......  ....................  ................................
#
     6.20%            27  emacs    emacs                 [.] decode_coding_utf_8
     4.67%            21  emacs    libc-2.27.so          [.] malloc_consolidate
     4.31%            19  emacs    libc-2.27.so          [.] __memmove_avx_unaligned_erms
     3.93%            17  emacs    emacs                 [.] produce_chars
     3.30%            15  emacs    libc-2.27.so          [.] malloc
     3.25%            15  emacs    libc-2.27.so          [.] _int_malloc
     2.95%            13  emacs    emacs                 [.] store_symval_forwarding
     2.78%            12  emacs    libc-2.27.so          [.] cfree@GLIBC_2.2.5
     2.75%            12  emacs    emacs                 [.] json_to_lisp
     2.70%             1  emacs    [kernel.kallsyms]     [k] sync_regs
     2.37%            12  emacs    emacs                 [.] mark_object
     2.08%             9  emacs    emacs                 [.] set_buffer_internal_2
     2.07%             9  emacs    emacs                 [.] make_uninit_multibyte_string
     2.06%             9  emacs    emacs                 [.] swap_in_symval_forwarding
     2.06%             9  emacs    emacs                 [.] do_symval_forwarding
     1.91%            10  emacs    emacs                 [.] sweep_strings
     1.55%             8  emacs    emacs                 [.] detect_coding
     1.44%             1  emacs    [kernel.kallsyms]     [k] error_entry
     1.36%             7  emacs    emacs                 [.] detect_coding_utf_8
     1.36%             6  emacs    emacs                 [.] hash_lookup
     1.34%             7  emacs    emacs                 [.] boyer_moore
     1.16%             5  emacs    libjansson.so.4.11.0  [.] json_delete
     1.14%             5  emacs    emacs                 [.] allocate_vectorlike
     1.13%             5  emacs    emacs                 [.] adjust_suspend_auto_hscroll
     1.08%             5  emacs    libjansson.so.4.11.0  [.] 0x0000000000007f40
     0.94%             4  emacs    emacs                 [.] insert_from_gap
     0.91%             4  emacs    libc-2.27.so          [.] __memchr_avx2
     0.89%             4  emacs    emacs                 [.] allocate_vector
     0.88%             4  emacs    libjansson.so.4.11.0  [.] 0x00000000000041f0
     0.84%             4  emacs    libjansson.so.4.11.0  [.] 0x00000000000044cf
     0.71%             3  emacs    libjansson.so.4.11.0  [.] 0x0000000000007f1f
     0.70%             3  emacs    libjansson.so.4.11.0  [.] json_array_get
     0.70%             3  emacs    emacs                 [.] make_buffer_string_both
     0.69%             3  emacs    emacs                 [.] Fcurrent_buffer
     0.69%             3  emacs    emacs                 [.] mem_insert
     0.69%             3  emacs    libjansson.so.4.11.0  [.] json_string_value
     0.68%             3  emacs    libjansson.so.4.11.0  [.] 0x00000000000041ec
     0.67%             3  emacs    emacs                 [.] Fnext_property_change
     0.67%             3  emacs    libjansson.so.4.11.0  [.] 0x00000000000041dc
     0.66%             3  emacs    emacs                 [.] decode_coding
     0.66%             3  emacs    emacs                 [.] json_malloc
     0.65%             3  emacs    libjansson.so.4.11.0  [.] 0x00000000000041f7
     0.63%             3  emacs    libjansson.so.4.11.0  [.] 0x00000000000041e4
     0.63%             3  emacs    libjansson.so.4.11.0  [.] 0x0000000000004096
     0.62%             1  emacs    [kernel.kallsyms]     [k] kmem_cache_alloc
     0.62%             3  emacs    libjansson.so.4.11.0  [.] 0x0000000000007e89
     0.61%             3  emacs    libjansson.so.4.11.0  [.] 0x00000000000040e0
     0.47%             2  emacs    emacs                 [.] invalidate_buffer_caches
     0.47%             2  emacs    emacs                 [.] allocate_string_data
     0.46%             2  emacs    emacs                 [.] del_range_2
     0.46%             2  emacs    emacs                 [.] assq_no_quit
     0.46%             2  emacs    emacs                 [.] code_convert_string
     0.46%             2  emacs    emacs                 [.] make_specified_string
     0.46%             2  emacs    libjansson.so.4.11.0  [.] 0x00000000000040a4
     0.46%             2  emacs    emacs                 [.] Fmake_local_variable
     0.46%             2  emacs    emacs                 [.] Fwiden
     0.45%             2  emacs    emacs                 [.] decode_coding_object
     0.45%             2  emacs    libjansson.so.4.11.0  [.] 0x0000000000003c1f
     0.44%             2  emacs    libjansson.so.4.11.0  [.] 0x00000000000040c5
     0.43%             2  emacs    libjansson.so.4.11.0  [.] 0x00000000000040bc
     0.43%             2  emacs    libjansson.so.4.11.0  [.] 0x0000000000004074
     0.42%             2  emacs    libjansson.so.4.11.0  [.] 0x0000000000007f53
     0.42%             2  emacs    libjansson.so.4.11.0  [.] 0x00000000000040ca
     0.41%             2  emacs    libjansson.so.4.11.0  [.] 0x000000000000408a
     0.41%             2  emacs    emacs                 [.] malloc@plt
     0.41%             2  emacs    libjansson.so.4.11.0  [.] 0x000000000000409e
     0.40%             2  emacs    libjansson.so.4.11.0  [.] json_array_append_new
     0.40%             2  emacs    emacs                 [.] pdumper_marked_p_impl
     0.39%             2  emacs    libjansson.so.4.11.0  [.] 0x0000000000008293
     0.37%             2  emacs    emacs                 [.] sweep_conses
     0.37%             2  emacs    emacs                 [.] sweep_vectors
     0.23%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000043c9
     0.23%             1  emacs    emacs                 [.] free@plt
     0.23%             1  emacs    emacs                 [.] json_array_size@plt
     0.23%             1  emacs    emacs                 [.] Fassq
     0.23%             1  emacs    emacs                 [.] Fcons
     0.23%             1  emacs    emacs                 [.] make_vector
     0.23%             1  emacs    emacs                 [.] do_one_unbind.constprop.24
     0.23%             1  emacs    emacs                 [.] hashfn_eq
     0.23%             1  emacs    emacs                 [.] json_array_get@plt
     0.23%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000040df
     0.23%             1  emacs    emacs                 [.] code_conversion_save
     0.23%             1  emacs    [kernel.kallsyms]     [k] rcu_nmi_enter
     0.23%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000007e82
     0.23%             1  emacs    emacs                 [.] set_buffer_internal_1
     0.23%             1  emacs    libjansson.so.4.11.0  [.] json_integer_value
     0.23%             1  emacs    emacs                 [.] json_free
     0.23%             1  emacs    emacs                 [.] setup_coding_system
     0.23%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000007e7d
     0.23%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000008290
     0.23%             1  emacs    emacs                 [.] record_insert
     0.23%             1  emacs    emacs                 [.] set_internal
     0.23%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000042e4
     0.23%             1  emacs    [kernel.kallsyms]     [k] mem_cgroup_try_charge
     0.23%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000041d0
     0.23%             1  emacs    emacs                 [.] validate_interval_range
     0.23%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000044e7
     0.23%             1  emacs    emacs                 [.] json_make_string
     0.23%             1  emacs    libjansson.so.4.11.0  [.] 0x000000000000483b
     0.23%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004260
     0.23%             1  emacs    libc-2.27.so          [.] __GI_____strtoll_l_internal
     0.23%             1  emacs    emacs                 [.] del_range_1
     0.23%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004314
     0.22%             1  emacs    emacs                 [.] coding_set_destination
     0.22%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004b80
     0.22%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000007f33
     0.22%             1  emacs    libjansson.so.4.11.0  [.] 0x000000000000407c
     0.22%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000040d1
     0.22%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000040e7
     0.22%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004091
     0.22%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004847
     0.21%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000040c0
     0.21%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004e0e
     0.21%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000007e74
     0.21%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000007f4e
     0.21%             1  emacs    libjansson.so.4.11.0  [.] 0x000000000000426a
     0.21%             1  emacs    emacs                 [.] balance_intervals
     0.21%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004290
     0.21%             1  emacs    emacs                 [.] unbind_to
     0.21%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000040db
     0.20%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000040b1
     0.20%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004854
     0.20%             1  emacs    libjansson.so.4.11.0  [.] 0x000000000000446e
     0.20%             1  emacs    emacs                 [.] pdumper_set_marked_impl
     0.20%             1  emacs    libjansson.so.4.11.0  [.] 0x000000000000452d
     0.20%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004271
     0.20%             1  emacs    emacs                 [.] mem_find.part.20
     0.19%             1  emacs    emacs                 [.] mem_delete.part.28
     0.18%             1  emacs    emacs                 [.] mark_vectorlike
     0.18%             1  emacs    emacs                 [.] vectorlike_nbytes
     0.00%             1  perf     [kernel.kallsyms]     [k] end_repeat_nmi
     0.00%             1  emacs    [kernel.kallsyms]     [k] native_sched_clock
     0.00%             3  perf     [kernel.kallsyms]     [k] native_write_msr
     0.00%             2  emacs    [kernel.kallsyms]     [k] native_write_msr
     0.00%             1  emacs    [unknown]             [.] 0000000000000000


#
# (Cannot load tips.txt file, please install perf!)
#

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: sebastien@chapu.is, yyoncho@gmail.com, 31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 16 Apr 2019 16:50:58 +0300
> 
> All right, my test script and results are attached. Here's how I run it:
> 
> sudo perf record -F 99 src/emacs -Q --batch -l ~/examples/elisp/json-test.el
> sudo perf report -n --stdio > perf-report.txt

Thanks.  I think we need to run this with a larger JSON, or maybe run
it several times in a loop.  Because there were too few samples in
this run, so we might see a biased picture due to overhead of stuff
that is done only once.

Also, please set gc-cons-threshold and gc-cons-percentage to very
large values, to prevent GC from biasing the profile even more.

> If you have certain recording or output format preferences, please let 
> me know.

The ASCII format is fine.

Was this an optimized build of Emacs or an unoptimized one?

bug#31138: Native json slower than json.el

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 1031 bytes --]

On 16.04.2019 18:13, Eli Zaretskii wrote:

> Thanks.  I think we need to run this with a larger JSON, or maybe run
> it several times in a loop.

There's a loop in benchmark already (10 times).

> Because there were too few samples in
> this run, so we might see a biased picture due to overhead of stuff
> that is done only once.

I've increased it to 100 (the process now takes 38 seconds here).

perf also has a sampling frequency parameter (still 99 Hz used).

> Also, please set gc-cons-threshold and gc-cons-percentage to very
> large values, to prevent GC from biasing the profile even more.

gc-cons-percentage says it's ignored when gc-cons-threshold is high 
enough. I've increased the latter.

>> If you have certain recording or output format preferences, please let
>> me know.
> 
> The ASCII format is fine.

It can add extra columns or some such.

> Was this an optimized build of Emacs or an unoptimized one?

What compiler should emacs be built with?               gcc -g3 -O2

The script and report are attached.

[-- Attachment #2: perf-report.txt --]
[-- Type: text/plain, Size: 25924 bytes --]

# To display the perf.data header info, please use --header/--header-only options.
#
#
# Total Lost Samples: 0
#
# Samples: 3K of event 'cycles:ppp'
# Event count (approx.): 120910650735
#
# Overhead       Samples  Command  Shared Object         Symbol                                        
# ........  ............  .......  ....................  ..............................................
#
     6.00%           229  emacs    emacs                 [.] decode_coding_utf_8
     5.31%           206  emacs    libc-2.27.so          [.] malloc
     4.57%           177  emacs    emacs                 [.] produce_chars
     4.12%           164  emacs    libc-2.27.so          [.] malloc_consolidate
     4.05%           161  emacs    libc-2.27.so          [.] cfree@GLIBC_2.2.5
     3.38%           135  emacs    libc-2.27.so          [.] __memmove_avx_unaligned_erms
     3.23%           125  emacs    emacs                 [.] store_symval_forwarding
     2.63%           102  emacs    emacs                 [.] do_symval_forwarding
     2.58%           104  emacs    libc-2.27.so          [.] _int_malloc
     2.58%           100  emacs    emacs                 [.] set_buffer_internal_2
     2.41%            93  emacs    emacs                 [.] allocate_vectorlike
     2.08%            81  emacs    emacs                 [.] json_to_lisp
     1.90%            81  emacs    emacs                 [.] detect_coding_utf_8
     1.85%            72  emacs    emacs                 [.] swap_in_symval_forwarding
     1.72%            72  emacs    emacs                 [.] detect_coding
     1.38%            31  emacs    emacs                 [.] insert_from_gap
     1.29%            50  emacs    libjansson.so.4.11.0  [.] json_delete
     1.19%            46  emacs    emacs                 [.] hash_lookup
     1.01%            39  emacs    emacs                 [.] decode_coding
     1.01%            39  emacs    libjansson.so.4.11.0  [.] json_array_get
     0.98%            45  emacs    emacs                 [.] sweep_strings
     0.95%            37  emacs    emacs                 [.] mem_insert
     0.95%            37  emacs    emacs                 [.] del_range_2
     0.85%            33  emacs    emacs                 [.] assq_no_quit
     0.77%            31  emacs    libc-2.27.so          [.] __memchr_avx2
     0.71%            29  emacs    emacs                 [.] boyer_moore
     0.67%            26  emacs    emacs                 [.] validate_interval_range
     0.61%            25  emacs    libjansson.so.4.11.0  [.] 0x00000000000041e4
     0.58%            23  emacs    libjansson.so.4.11.0  [.] 0x00000000000041dc
     0.57%            23  emacs    libjansson.so.4.11.0  [.] 0x00000000000040a4
     0.57%            23  emacs    libjansson.so.4.11.0  [.] 0x0000000000007f33
     0.57%            23  emacs    libjansson.so.4.11.0  [.] 0x0000000000003c61
     0.57%            23  emacs    libjansson.so.4.11.0  [.] 0x0000000000004847
     0.56%            22  emacs    emacs                 [.] allocate_string_data
     0.53%            21  emacs    libjansson.so.4.11.0  [.] 0x0000000000007f58
     0.53%            21  emacs    libjansson.so.4.11.0  [.] 0x0000000000003c1f
     0.52%            21  emacs    libjansson.so.4.11.0  [.] 0x0000000000004074
     0.52%            20  emacs    emacs                 [.] code_convert_string
     0.52%            20  emacs    emacs                 [.] make_uninit_multibyte_string
     0.51%            21  emacs    libjansson.so.4.11.0  [.] 0x00000000000041f0
     0.51%            20  emacs    emacs                 [.] decode_coding_object
     0.50%            20  emacs    libjansson.so.4.11.0  [.] 0x00000000000041c5
     0.50%            20  emacs    libjansson.so.4.11.0  [.] 0x0000000000007f53
     0.50%            20  emacs    emacs                 [.] json_malloc
     0.50%            20  emacs    libjansson.so.4.11.0  [.] 0x000000000000483b
     0.49%            19  emacs    emacs                 [.] del_range_1
     0.47%            19  emacs    libjansson.so.4.11.0  [.] 0x000000000000408a
     0.47%            19  emacs    libjansson.so.4.11.0  [.] 0x0000000000007f1f
     0.46%            18  emacs    emacs                 [.] setup_coding_system
     0.46%            18  emacs    libjansson.so.4.11.0  [.] json_string_value
     0.44%            18  emacs    libjansson.so.4.11.0  [.] 0x0000000000007e7d
     0.44%            17  emacs    emacs                 [.] adjust_suspend_auto_hscroll
     0.44%            17  emacs    emacs                 [.] invalidate_buffer_caches
     0.42%            17  emacs    libjansson.so.4.11.0  [.] 0x00000000000041ec
     0.42%            17  emacs    libjansson.so.4.11.0  [.] memcpy@plt
     0.42%            16  emacs    [kernel.kallsyms]     [k] clear_page_erms
     0.41%            17  emacs    libjansson.so.4.11.0  [.] 0x0000000000007f17
     0.41%            16  emacs    emacs                 [.] Fsymbol_value
     0.41%            16  emacs    emacs                 [.] prepare_to_modify_buffer_1
     0.40%            16  emacs    libjansson.so.4.11.0  [.] 0x00000000000044e7
     0.39%            16  emacs    libjansson.so.4.11.0  [.] 0x0000000000004096
     0.39%            15  emacs    emacs                 [.] find_symbol_value
     0.38%            18  emacs    emacs                 [.] sweep_vectors
     0.37%            15  emacs    libjansson.so.4.11.0  [.] 0x00000000000040b1
     0.37%            15  emacs    libjansson.so.4.11.0  [.] json_array_append_new
     0.37%            15  emacs    libjansson.so.4.11.0  [.] 0x0000000000004091
     0.37%            15  emacs    libjansson.so.4.11.0  [.] 0x000000000000407c
     0.36%            14  emacs    libjansson.so.4.11.0  [.] 0x0000000000007e89
     0.35%            14  emacs    libjansson.so.4.11.0  [.] 0x00000000000041f7
     0.35%            14  emacs    libjansson.so.4.11.0  [.] 0x00000000000040ca
     0.35%            14  emacs    libjansson.so.4.11.0  [.] 0x00000000000040c5
     0.34%            14  emacs    libjansson.so.4.11.0  [.] 0x00000000000044b8
     0.34%            13  emacs    emacs                 [.] make_buffer_string_both
     0.32%            13  emacs    libjansson.so.4.11.0  [.] 0x000000000000409e
     0.32%            13  emacs    libjansson.so.4.11.0  [.] 0x00000000000047fe
     0.32%            13  emacs    libjansson.so.4.11.0  [.] 0x00000000000040e0
     0.31%            12  emacs    [kernel.kallsyms]     [k] get_page_from_freelist
     0.31%            12  emacs    emacs                 [.] set_internal
     0.30%            12  emacs    libjansson.so.4.11.0  [.] 0x0000000000007f40
     0.29%            12  emacs    libjansson.so.4.11.0  [.] 0x0000000000007e74
     0.29%            12  emacs    libjansson.so.4.11.0  [.] 0x0000000000007f07
     0.29%            11  emacs    [kernel.kallsyms]     [k] sync_regs
     0.26%            10  emacs    [kernel.kallsyms]     [k] swapgs_restore_regs_and_return_to_usermode
     0.26%            10  emacs    libjansson.so.4.11.0  [.] json_string_length
     0.25%            10  emacs    [kernel.kallsyms]     [k] copy_user_enhanced_fast_string
     0.23%             9  emacs    emacs                 [.] Fnext_property_change
     0.23%             9  emacs    emacs                 [.] make_vector
     0.22%             9  emacs    libjansson.so.4.11.0  [.] 0x00000000000040db
     0.22%             9  emacs    libjansson.so.4.11.0  [.] 0x00000000000040d6
     0.22%             9  emacs    libjansson.so.4.11.0  [.] 0x00000000000044cf
     0.21%            10  emacs    emacs                 [.] mem_delete.part.28
     0.21%             8  emacs    emacs                 [.] coding_set_destination
     0.21%             8  emacs    emacs                 [.] Ferase_buffer
     0.20%             8  emacs    libjansson.so.4.11.0  [.] json_array_size
     0.20%             1  emacs    [kernel.kallsyms]     [k] __atime_needs_update
     0.20%             8  emacs    libjansson.so.4.11.0  [.] 0x0000000000008290
     0.20%             8  emacs    libjansson.so.4.11.0  [.] 0x00000000000040ba
     0.19%             1  gmain    [kernel.kallsyms]     [k] acct_collect
     0.18%             7  emacs    emacs                 [.] make_specified_string
     0.18%             7  emacs    emacs                 [.] unbind_to
     0.18%             7  emacs    emacs                 [.] record_buffer_markers
     0.18%             7  emacs    emacs                 [.] json_make_string
     0.18%             7  emacs    emacs                 [.] json_free
     0.18%             7  emacs    emacs                 [.] record_insert
     0.17%             7  emacs    libjansson.so.4.11.0  [.] 0x00000000000040bc
     0.17%             7  emacs    libjansson.so.4.11.0  [.] 0x000000000000829a
     0.16%             8  emacs    emacs                 [.] mem_find.part.20
     0.16%             6  emacs    emacs                 [.] recenter_overlay_lists
     0.15%             6  emacs    emacs                 [.] do_one_unbind.constprop.24
     0.15%             6  emacs    libjansson.so.4.11.0  [.] json_integer_value
     0.15%             6  emacs    emacs                 [.] Fassq
     0.15%             6  emacs    emacs                 [.] update_compositions
     0.15%             6  emacs    libjansson.so.4.11.0  [.] 0x00000000000040e3
     0.15%             6  emacs    [kernel.kallsyms]     [k] _raw_spin_lock
     0.15%             6  emacs    libjansson.so.4.11.0  [.] 0x00000000000040d1
     0.15%             6  emacs    libjansson.so.4.11.0  [.] 0x00000000000040e5
     0.13%             5  emacs    emacs                 [.] fetch_buffer_markers
     0.13%             5  emacs    emacs                 [.] Fcons
     0.13%             5  emacs    emacs                 [.] Fwiden
     0.13%             5  emacs    emacs                 [.] Fmake_local_variable
     0.13%             5  emacs    [kernel.kallsyms]     [k] error_entry
     0.13%             5  emacs    emacs                 [.] offset_intervals
     0.13%             5  emacs    emacs                 [.] adjust_overlays_for_delete
     0.13%             5  emacs    libjansson.so.4.11.0  [.] 0x0000000000007f4e
     0.13%             5  emacs    libjansson.so.4.11.0  [.] 0x0000000000004098
     0.12%             5  emacs    libjansson.so.4.11.0  [.] 0x00000000000040cf
     0.12%             5  emacs    libjansson.so.4.11.0  [.] 0x0000000000003c1e
     0.12%             5  emacs    libjansson.so.4.11.0  [.] 0x0000000000004806
     0.10%             4  emacs    emacs                 [.] buf_charpos_to_bytepos
     0.10%             4  emacs    [kernel.kallsyms]     [k] native_irq_return_iret
     0.10%             4  emacs    emacs                 [.] free@plt
     0.10%             4  emacs    emacs                 [.] get_translation_table
     0.10%             4  emacs    [kernel.kallsyms]     [k] __pagevec_lru_add_fn
     0.10%             4  emacs    libjansson.so.4.11.0  [.] 0x00000000000040e7
     0.10%             4  emacs    libjansson.so.4.11.0  [.] 0x0000000000007e7a
     0.10%             4  emacs    libjansson.so.4.11.0  [.] 0x00000000000056a5
     0.10%             4  emacs    libjansson.so.4.11.0  [.] 0x0000000000004b30
     0.10%             4  emacs    libjansson.so.4.11.0  [.] 0x00000000000042c3
     0.10%             4  emacs    libjansson.so.4.11.0  [.] 0x0000000000007e85
     0.10%             4  emacs    libjansson.so.4.11.0  [.] 0x0000000000007e77
     0.10%             4  emacs    libjansson.so.4.11.0  [.] 0x00000000000040c0
     0.08%             3  emacs    [kernel.kallsyms]     [k] __alloc_pages_nodemask
     0.08%             3  gmain    [kernel.kallsyms]     [k] release_pages
     0.08%             3  emacs    emacs                 [.] json_array_size@plt
     0.08%             3  emacs    [kernel.kallsyms]     [k] handle_mm_fault
     0.08%             3  emacs    [kernel.kallsyms]     [k] alloc_pages_vma
     0.08%             3  emacs    emacs                 [.] update_buffer_properties
     0.08%             3  emacs    emacs                 [.] CHECK_STRING_OR_BUFFER
     0.08%             3  emacs    emacs                 [.] Ftext_properties_at
     0.08%             3  emacs    emacs                 [.] code_conversion_save
     0.08%             3  emacs    libjansson.so.4.11.0  [.] 0x0000000000004854
     0.08%             3  emacs    libjansson.so.4.11.0  [.] 0x00000000000040df
     0.08%             3  emacs    libjansson.so.4.11.0  [.] 0x0000000000003c69
     0.08%             3  emacs    libjansson.so.4.11.0  [.] 0x00000000000040e2
     0.08%             3  emacs    libjansson.so.4.11.0  [.] 0x0000000000004440
     0.08%             3  emacs    libjansson.so.4.11.0  [.] 0x0000000000007f49
     0.08%             3  emacs    libjansson.so.4.11.0  [.] 0x0000000000007f30
     0.08%             3  emacs    emacs                 [.] del_range
     0.07%             3  emacs    libjansson.so.4.11.0  [.] 0x0000000000007e7e
     0.07%             3  emacs    emacs                 [.] malloc@plt
     0.07%             3  emacs    libjansson.so.4.11.0  [.] 0x0000000000004469
     0.07%             3  emacs    libjansson.so.4.11.0  [.] 0x0000000000004ade
     0.07%             3  emacs    libjansson.so.4.11.0  [.] 0x00000000000043eb
     0.07%             3  emacs    libjansson.so.4.11.0  [.] 0x00000000000040e9
     0.07%             3  emacs    libjansson.so.4.11.0  [.] 0x0000000000004e1c
     0.07%             3  emacs    libjansson.so.4.11.0  [.] 0x0000000000008293
     0.05%             2  emacs    emacs                 [.] grow_specpdl
     0.05%             2  emacs    emacs                 [.] invalidate_current_column
     0.05%             2  emacs    libc-2.27.so          [.] sysmalloc
     0.05%             2  emacs    emacs                 [.] Fcurrent_buffer
     0.05%             2  emacs    emacs                 [.] json_string_length@plt
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x0000000000004366
     0.05%             2  emacs    emacs                 [.] signal_after_change
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x0000000000004ac0
     0.05%             3  emacs    emacs                 [.] sweep_conses
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x0000000000007e4e
     0.05%             2  emacs    emacs                 [.] evaporate_overlays
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x000000000000446e
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x0000000000004076
     0.05%             2  emacs    emacs                 [.] prepare_to_modify_buffer
     0.05%             2  emacs    emacs                 [.] lisp_align_malloc
     0.05%             2  emacs    emacs                 [.] allocate_vector
     0.05%             2  emacs    emacs                 [.] set_buffer_internal_1
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x0000000000008502
     0.05%             2  emacs    libc-2.27.so          [.] __GI_____strtoll_l_internal
     0.05%             2  emacs    [kernel.kallsyms]     [k] lru_cache_add_active_or_unevictable
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x0000000000007f1d
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x0000000000003c10
     0.05%             2  emacs    emacs                 [.] setup_on_free_list
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x0000000000004443
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x00000000000042d3
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x0000000000004084
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x00000000000043e0
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x000000000000430d
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x0000000000004399
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x0000000000007f44
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x00000000000042f2
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x00000000000042ce
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x0000000000007e70
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x0000000000004845
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x000000000000439b
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x00000000000042e9
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x000000000000407f
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x0000000000004284
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x0000000000004260
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x0000000000007f79
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x0000000000003c68
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x0000000000004abd
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x0000000000004e0e
     0.05%             2  emacs    libjansson.so.4.11.0  [.] 0x00000000000041c0
     0.05%             2  emacs    emacs                 [.] vectorlike_nbytes
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004270
     0.03%             1  emacs    [kernel.kallsyms]     [k] mem_cgroup_page_lruvec
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004e29
     0.03%             1  gmain    [kernel.kallsyms]     [k] unmap_page_range
     0.03%             1  emacs    [kernel.kallsyms]     [k] mem_cgroup_try_charge
     0.03%             1  emacs    emacs                 [.] adjust_markers_for_insert
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004b35
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000084f0
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000007e92
     0.03%             1  emacs    emacs                 [.] json_string_value@plt
     0.03%             1  emacs    [kernel.kallsyms]     [k] _cond_resched
     0.03%             1  emacs    [kernel.kallsyms]     [k] page_evictable
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x000000000000484b
     0.03%             1  emacs    emacs                 [.] bset_redisplay
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004ac4
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x000000000000428b
     0.03%             1  emacs    [kernel.kallsyms]     [k] page_mapping
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004862
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000041c1
     0.03%             1  emacs    [kernel.kallsyms]     [k] find_vma
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000008298
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000007e79
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004b4e
     0.03%             1  emacs    [kernel.kallsyms]     [k] __do_page_fault
     0.03%             1  emacs    [kernel.kallsyms]     [k] cap_capable
     0.03%             1  emacs    [kernel.kallsyms]     [k] wq_worker_waking_up
     0.03%             1  emacs    [kernel.kallsyms]     [k] timekeeping_update
     0.03%             1  emacs    libjansson.so.4.11.0  [.] memchr@plt
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000008516
     0.03%             1  emacs    [kernel.kallsyms]     [k] mem_cgroup_commit_charge
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004200
     0.03%             1  emacs    [kernel.kallsyms]     [k] idle_cpu
     0.03%             1  emacs    emacs                 [.] maybe_quit
     0.03%             1  emacs    [kernel.kallsyms]     [k] apic_timer_interrupt
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x000000000000408d
     0.03%             1  emacs    [kernel.kallsyms]     [k] get_mem_cgroup_from_mm
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000042f0
     0.03%             1  emacs    emacs                 [.] Fbuffer_live_p
     0.03%             1  emacs    emacs                 [.] record_delete
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004840
     0.03%             1  emacs    [kernel.kallsyms]     [k] arch_get_unmapped_area_topdown
     0.03%             1  emacs    [kernel.kallsyms]     [k] up_read
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000056b0
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004323
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000007f04
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000043b2
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x000000000000452d
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000041c9
     0.03%             1  emacs    emacs                 [.] record_unwind_protect
     0.03%             1  emacs    [kernel.kallsyms]     [k] __accumulate_pelt_segments
     0.03%             1  emacs    [kernel.kallsyms]     [k] policy_node
     0.03%             1  emacs    [kernel.kallsyms]     [k] try_charge
     0.03%             1  emacs    [kernel.kallsyms]     [k] __handle_mm_fault
     0.03%             1  emacs    libjansson.so.4.11.0  [.] json_null
     0.03%             1  emacs    [kernel.kallsyms]     [k] get_task_policy.part.30
     0.03%             1  emacs    emacs                 [.] coding_set_source
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004290
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000041f5
     0.03%             1  emacs    [kernel.kallsyms]     [k] generic_file_read_iter
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x000000000000427b
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000040ac
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000045ae
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000042dc
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004093
     0.03%             1  emacs    [kernel.kallsyms]     [k] wbc_detach_inode
     0.03%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000084f6
     0.02%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000003c22
     0.02%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000044a3
     0.02%             1  emacs    [kernel.kallsyms]     [k] syscall_return_via_sysret
     0.02%             1  emacs    [kernel.kallsyms]     [k] common_file_perm
     0.02%             1  emacs    emacs                 [.] adjust_overlays_for_insert
     0.02%             1  emacs    [kernel.kallsyms]     [k] reweight_entity
     0.02%             1  emacs    [kernel.kallsyms]     [k] __entry_trampoline_start
     0.02%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004ab4
     0.02%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000043e3
     0.02%             1  emacs    [kvm]                 [k] pvclock_gtod_notify
     0.02%             1  emacs    emacs                 [.] json_integer_value@plt
     0.02%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004077
     0.02%             1  emacs    [kernel.kallsyms]     [k] account_user_time
     0.02%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004bec
     0.02%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004e31
     0.02%             1  emacs    emacs                 [.] re_match_2_internal
     0.02%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000045b3
     0.02%             1  gmain    [kernel.kallsyms]     [k] free_pages_and_swap_cache
     0.02%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004072
     0.02%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000043ed
     0.02%             1  emacs    emacs                 [.] exec_byte_code
     0.02%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004268
     0.02%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000007e4b
     0.02%             1  emacs    libjansson.so.4.11.0  [.] 0x000000000000482c
     0.02%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004593
     0.02%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000040b5
     0.02%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004b75
     0.02%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004277
     0.02%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000089e5
     0.02%             1  emacs    libjansson.so.4.11.0  [.] 0x00000000000044e4
     0.02%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004314
     0.02%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000007e82
     0.02%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000008512
     0.02%             1  emacs    libjansson.so.4.11.0  [.] 0x0000000000004070
     0.02%             1  emacs    libjansson.so.4.11.0  [.] json_array
     0.02%             1  emacs    libjansson.so.4.11.0  [.] 0x000000000000433e
     0.02%             1  emacs    emacs                 [.] lisp_align_free
     0.00%             1  perf     [kernel.kallsyms]     [k] end_repeat_nmi
     0.00%             1  emacs    [kernel.kallsyms]     [k] end_repeat_nmi
     0.00%             1  emacs    [kernel.kallsyms]     [k] nmi_cpu_backtrace
     0.00%             6  emacs    [kernel.kallsyms]     [k] native_write_msr
     0.00%             2  perf     [kernel.kallsyms]     [k] native_write_msr
     0.00%             1  perf     [unknown]             [.] 0000000000000000


#
# (Cannot load tips.txt file, please install perf!)
#

[-- Attachment #3: json-test.el --]
[-- Type: text/x-emacs-lisp, Size: 227 bytes --]

(defun test ()
  (with-temp-buffer
    (insert-file-contents "~/examples/sample-response.json")
    (goto-char (point-min))
    (json-parse-buffer)))

(let ((gc-cons-threshold most-positive-fixnum))
  (benchmark 100 '(test)))

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: sebastien@chapu.is, yyoncho@gmail.com, 31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 16 Apr 2019 18:30:08 +0300
> 
> The script and report are attached.

Thanks, can you also show the sample-response.json file?

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 16.04.2019 19:10, Eli Zaretskii wrote:
>> Cc: sebastien@chapu.is, yyoncho@gmail.com, 31138@debbugs.gnu.org
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Date: Tue, 16 Apr 2019 18:30:08 +0300
>>
>> The script and report are attached.
> 
> Thanks, can you also show the sample-response.json file?

It's an output from a code assistance tool listing all classes in our 
internal application, including libraries. I probably shouldn't 
distribute it.

But it's basically a long list of entries like:

[["Gem::Licenses",null,"suggestions",[["req","license"]],"/home/dgutov/.rbenv/versions/2.6.1/lib/ruby/2.6.0/rubygems/util/licenses.rb",429],["Gem::Licenses",null,"match?",[["req","license"]],"/home/dgutov/.rbenv/versions/2.6.1/lib/ruby/2.6.0/rubygems/util/licenses.rb",425],["URI::Generic",null,"use_proxy?",[["req","hostname"],["req","addr"],["req","port"],["req","no_proxy"]],"/home/dgutov/.rbenv/versions/2.6.1/lib/ruby/2.6.0/uri/generic.rb",1545],["URI::Generic",null,"default_port",[],"/home/dgutov/.rbenv/versions/2.6.1/lib/ruby/2.6.0/uri/generic.rb",33],["URI::Generic",null,"component",[],"/home/dgutov/.rbenv/versions/2.6.1/lib/ruby/2.6.0/uri/generic.rb",58], 
...

If you think it would help, I can anonymize it and send the result, but 
large.json posted by Ivan here previously shows the same performance 
characteristics, I think.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: sebastien@chapu.is, yyoncho@gmail.com, 31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 16 Apr 2019 19:23:11 +0300
> 
> If you think it would help, I can anonymize it and send the result

No need for now.

Thanks, I will look into the results some more soon.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Date: Tue, 16 Apr 2019 19:44:08 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: sebastien@chapu.is, yyoncho@gmail.com, 31138@debbugs.gnu.org
> 
> > Cc: sebastien@chapu.is, yyoncho@gmail.com, 31138@debbugs.gnu.org
> > From: Dmitry Gutov <dgutov@yandex.ru>
> > Date: Tue, 16 Apr 2019 19:23:11 +0300
> > 
> > If you think it would help, I can anonymize it and send the result
> 
> No need for now.
> 
> Thanks, I will look into the results some more soon.

I profiled the code, and my results are very similar to yours.  My
interpretation of the results is that decoding strings that we receive
from the jansson library takes less than 10% of the CPU time.  So I
think we don't need to worry about this aspect when considering
parsing of JSON objects.

I think we can close this bug now.

Thanks.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 21.04.2019 11:58, Eli Zaretskii wrote:
> I profiled the code, and my results are very similar to yours.  My
> interpretation of the results is that decoding strings that we receive
> from the jansson library takes less than 10% of the CPU time.  So I
> think we don't need to worry about this aspect when considering
> parsing of JSON objects.

Eli, how did you reach that conclusion?

One: looking at the profile, libjansson is somewhere down below, and 
most of the runtime is taken by Emacs.

Two: while I might be misreading the profile, simply omitting the 
decoding function call (and using the returned strings as-is) halves the 
execution time.

How could that be compatible with your interpretation?

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: 31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sun, 21 Apr 2019 12:15:59 +0300
> 
> On 21.04.2019 11:58, Eli Zaretskii wrote:
> > I profiled the code, and my results are very similar to yours.  My
> > interpretation of the results is that decoding strings that we receive
> > from the jansson library takes less than 10% of the CPU time.  So I
> > think we don't need to worry about this aspect when considering
> > parsing of JSON objects.
> 
> Eli, how did you reach that conclusion?

By adding the percentage of all the functions involved in decoding
strings.

> One: looking at the profile, libjansson is somewhere down below, and 
> most of the runtime is taken by Emacs.

I only accounted for time used by Emacs code, i.e. 100% doesn't
include the library.

> Two: while I might be misreading the profile, simply omitting the 
> decoding function call (and using the returned strings as-is) halves the 
> execution time.

You cannot use the returned string as is, you must make a Lisp string
out of it.

In any case, even if decoding takes 50% of the time we spend in Emacs
code, it is still not significant enough to justify the un-safety of
using a string that we didn't decode, because if that string ever
includes raw bytes, Emacs will surely crash.

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 477 bytes --]

Hi Eli,


> In any case, even if decoding takes 50% of the time we spend in Emacs
> code, it is still not significant enough to justify the un-safety of
> using a string that we didn't decode, because if that string ever
> includes raw bytes, Emacs will surely crash.
>

IMO if validation of the data that comes from external libraries is taking
a lot of time then it does not make sense to use external libraries at all
when we are aiming for performance gains.

Thanks,
Ivan

[-- Attachment #2: Type: text/html, Size: 781 bytes --]

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Sun, 21 Apr 2019 13:23:28 +0300
> Cc: Dmitry Gutov <dgutov@yandex.ru>, Sébastien Chapuis <sebastien@chapu.is>, 
> 	31138@debbugs.gnu.org
> 
> IMO if validation of the data that comes from external libraries is taking a lot of time then it does not make
> sense to use external libraries at all when we are aiming for performance gains.

I don't see how this follows, when this discussion clearly established
that native JSON parsing is much faster than parsing in Lisp.

I also don't see where did "a lot of time" come from, I think it's an
exaggeration, given the processing speed shown by benchmarks in this
discussion.

There's still some peculiar slowdown in your configuration, which
AFAIR couldn't be reproduced from "emacs -Q"; I still hope you will be
able to find what causes that.

As for the more general point: we always decode strings, even in UTF-8
locales, so I don't see how this case is different.

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 768 bytes --]

Hi Eli,

I don't see how this follows, when this discussion clearly established
> that native JSON parsing is much faster than parsing in Lisp.
>

We also established that current native performance is not fast enough to
handle real-world scenarios.


> I also don't see where did "a lot of time" come from, I think it's an
> exaggeration, given the processing speed shown by benchmarks in this
> discussion.
>

I said that *if* this is the case. You said that if 50% is spent in
decoding it is not enough to justify the risk of crashing emacs. What I am
saying is that IMO if 50% of the time is spent in decoding data that comes
from Jansson it will be enough to justify not using Jansson at all and
writing the whole Json parsing in Emacs Source tree.

Thanks,
Ivan

[-- Attachment #2: Type: text/html, Size: 1244 bytes --]

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Sun, 21 Apr 2019 14:38:04 +0300
> Cc: Dmitry Gutov <dgutov@yandex.ru>, Sébastien Chapuis <sebastien@chapu.is>, 
> 	31138@debbugs.gnu.org
> 
>  I don't see how this follows, when this discussion clearly established
>  that native JSON parsing is much faster than parsing in Lisp.
> 
> We also established that current native performance is not fast enough to handle real-world scenarios. 

Then maybe we should look for a faster JSON parsing library, because
libjansson functions appear in the profile as high or higher than our
decoding routines.  Or maybe this is the limit of what can be done in
Emacs, since there is, and always will be, overhead due to converting
C values to Lisp objects.

>  I also don't see where did "a lot of time" come from, I think it's an
>  exaggeration, given the processing speed shown by benchmarks in this
>  discussion.
> 
> I said that *if* this is the case. You said that if 50% is spent in decoding it is not enough to justify the risk of
> crashing emacs. What I am saying is that IMO if 50% of the time is spent in decoding data that comes from
> Jansson it will be enough to justify not using Jansson at all and writing the whole Json parsing in Emacs
> Source tree.

I don't think I understand how this could help to speed up code,
please explain.

The issue here is not that the library is some code not written by the
Emacs project, because we do the same with text generated by Emacs
Lisp programs.  Bugs can happen in both external libraries and in code
written by Emacs programmers.

bug#31138: Native json slower than json.el

[Philipp Stephani]

Am So., 21. Apr. 2019 um 12:24 Uhr schrieb yyoncho <yyoncho@gmail.com>:
>
> Hi Eli,
>
>>
>> In any case, even if decoding takes 50% of the time we spend in Emacs
>> code, it is still not significant enough to justify the un-safety of
>> using a string that we didn't decode, because if that string ever
>> includes raw bytes, Emacs will surely crash.
>
>
> IMO if validation of the data that comes from external libraries is taking a lot of time then it does not make sense to use external libraries at all when we are aiming for performance gains.


There's no need to validate Jansson's output because it already
validates that the input is valid UTF-8, and never generates invalid
strings.

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 960 bytes --]

Hi Philipp,

As Eli commented in this thread, the decoding is must have for all external
libraries no matter what is their contract.

Thanks,
Ivan

On Sun, Apr 21, 2019 at 3:59 PM Philipp Stephani <p.stephani2@gmail.com>
wrote:

> Am So., 21. Apr. 2019 um 12:24 Uhr schrieb yyoncho <yyoncho@gmail.com>:
> >
> > Hi Eli,
> >
> >>
> >> In any case, even if decoding takes 50% of the time we spend in Emacs
> >> code, it is still not significant enough to justify the un-safety of
> >> using a string that we didn't decode, because if that string ever
> >> includes raw bytes, Emacs will surely crash.
> >
> >
> > IMO if validation of the data that comes from external libraries is
> taking a lot of time then it does not make sense to use external libraries
> at all when we are aiming for performance gains.
>
>
> There's no need to validate Jansson's output because it already
> validates that the input is valid UTF-8, and never generates invalid
> strings.
>

[-- Attachment #2: Type: text/html, Size: 1468 bytes --]

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 1317 bytes --]

Hi Eli,


> Or maybe this is the limit of what can be done in
> Emacs, since there is, and always will be, overhead due to converting
> C values to Lisp objects.
>

Not necessary, there are libraries (at least in java/rust) which provide a
possibility to pass a generator which can be used to create your data
structures on the fly to avoid intermittent structures. This is something
that should be considered when looking for C json library.


> I don't think I understand how this could help to speed up code,
> please explain.
>
> The issue here is not that the library is some code not written by the
> Emacs project, because we do the same with text generated by Emacs
> Lisp programs.  Bugs can happen in both external libraries and in code
> written by Emacs programmers.
>

I am a bit confused, (maybe I am talking nonsense) but if the input is
already an emacs lisp string doesn't that mean that we should not decode it
since it is already validated? But even if we had to convert it, we can do
it once(e. g. apply decode string on the input string) instead of doing
that for each string in the JSON and eventually yield performance
improvement? For a reference, I have compared the performance of
read-from-string with the json equivalent and it is ~3 times faster(if
memory serves me right).

Thanks,
Ivan

[-- Attachment #2: Type: text/html, Size: 1795 bytes --]

bug#31138: Native json slower than json.el

[Philipp Stephani]

Am So., 21. Apr. 2019 um 15:10 Uhr schrieb yyoncho <yyoncho@gmail.com>:
>
> Hi Philipp,
>
> As Eli commented in this thread, the decoding is must have for all external libraries no matter what is their contract.

I disagree.

>
> Thanks,
> Ivan
>
> On Sun, Apr 21, 2019 at 3:59 PM Philipp Stephani <p.stephani2@gmail.com> wrote:
>>
>> Am So., 21. Apr. 2019 um 12:24 Uhr schrieb yyoncho <yyoncho@gmail.com>:
>> >
>> > Hi Eli,
>> >
>> >>
>> >> In any case, even if decoding takes 50% of the time we spend in Emacs
>> >> code, it is still not significant enough to justify the un-safety of
>> >> using a string that we didn't decode, because if that string ever
>> >> includes raw bytes, Emacs will surely crash.
>> >
>> >
>> > IMO if validation of the data that comes from external libraries is taking a lot of time then it does not make sense to use external libraries at all when we are aiming for performance gains.
>>
>>
>> There's no need to validate Jansson's output because it already
>> validates that the input is valid UTF-8, and never generates invalid
>> strings.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Sun, 21 Apr 2019 16:28:02 +0300
> Cc: Dmitry Gutov <dgutov@yandex.ru>, Sébastien Chapuis <sebastien@chapu.is>, 
> 	31138@debbugs.gnu.org
> 
>  The issue here is not that the library is some code not written by the
>  Emacs project, because we do the same with text generated by Emacs
>  Lisp programs.  Bugs can happen in both external libraries and in code
>  written by Emacs programmers.
> 
> I am a bit confused, (maybe I am talking nonsense) but if the input is already an emacs lisp string doesn't that
> mean that we should not decode it since it is already validated?

If the text is multibyte text, yes.  But Lisp programs can also
generate unibyte text.

> But even if we had to convert it, we can do it
> once(e. g. apply decode string on the input string) instead of doing that for each string in the JSON and
> eventually yield performance improvement?

We can do that, yes.  I don't know what kind of speedup this will get
us, we should measure that.

> For a reference, I have compared the performance of read-from-string
> with the json equivalent and it is ~3 times faster(if memory serves
> me right).

read-from-string doesn't decode.  You need to time
decode-coding-string when the string is the entire JSON.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Date: Sun, 21 Apr 2019 22:03:07 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: sebastien@chapu.is, dgutov@yandex.ru, 31138@debbugs.gnu.org
> 
> > But even if we had to convert it, we can do it
> > once(e. g. apply decode string on the input string) instead of doing that for each string in the JSON and
> > eventually yield performance improvement?
> 
> We can do that, yes.

Actually, I think I was confused regarding your question: the input
string in the use cases we were discussing was already decoded.  We
encode it before passing it to the jansson library.

So I don't see how we could avoid decoding each string we receive from
the library.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 21.04.2019 12:31, Eli Zaretskii wrote:

>> Eli, how did you reach that conclusion?
> 
> By adding the percentage of all the functions involved in decoding
> strings.

Was the total sum of the profile 100%?

>> One: looking at the profile, libjansson is somewhere down below, and
>> most of the runtime is taken by Emacs.
> 
> I only accounted for time used by Emacs code, i.e. 100% doesn't
> include the library.

I still don't understand. I my profiling report, decode_coding_utf_8 is 
at the top (Emacs function). Followed by malloc and produce_chars. Are 
you saying they are not involved in decoding strings?

>> Two: while I might be misreading the profile, simply omitting the
>> decoding function call (and using the returned strings as-is) halves the
>> execution time.
> 
> You cannot use the returned string as is, you must make a Lisp string
> out of it.

Yes. There's a patch that makes a Lisp string without string additional 
decoding, and it makes the runtime 2x faster. It's been posted in this 
report. Would you like me to point at it?

> In any case, even if decoding takes 50% of the time we spend in Emacs
> code, it is still not significant enough to justify the un-safety of
> using a string that we didn't decode, because if that string ever
> includes raw bytes, Emacs will surely crash.

What was the point of us doing the exercise, and me wasting time on it, 
if you're set on not changing anything at all?

We've already had the performance numbers from before.

Given the encouragement, I really expected you to choose *some* path 
toward improvement. Either forgoing conversion (maybe adding an 
additional test suite, for cases you're worrying about), or doing some 
kind of faster validation instead of full conversion (where we don't 
allocate new strings but just check existing ones for validity), or, you 
know, optimizing everything everywhere.

Considering libjansson manages to do both JSON parsing and string 
conversion in ~the same time make_specified_string only does string 
conversion on the returned strings, it most likely follows that 
make_specified_string could be made faster.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 21.04.2019 13:23, yyoncho wrote:
> IMO if validation of the data that comes from external libraries is 
> taking a lot of time then it does not make sense to use external 
> libraries at all when we are aiming for performance gains.

If we'll have to call make_specified_string for each parsed JSON string, 
and its speed is unchanged, I doubt that would help much still. Only as 
much as 2x, most likely less.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 21.04.2019 15:15, Eli Zaretskii wrote:

> Then maybe we should look for a faster JSON parsing library, because
> libjansson functions appear in the profile as high or higher than our
> decoding routines.

Not in my profile.

Again, if make_specified_string takes 50% of the runtime, even if 
libjansson (or some other JSON lib) worked instantly, we'd only get a 2x 
improvement at most.

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 318 bytes --]

Hi Eli,

read-from-string doesn't decode.  You need to time
> decode-coding-string when the string is the entire JSON.
>

Can you elaborate? Both json-parse-string and read-from-string do the same
from my point of view -> convert string to elisp data structure with
read-from-string being 3times faster.

Thanks,
Ivan

[-- Attachment #2: Type: text/html, Size: 624 bytes --]

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: sebastien@chapu.is, yyoncho@gmail.com, 31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 22 Apr 2019 01:12:49 +0300
> 
> On 21.04.2019 12:31, Eli Zaretskii wrote:
> 
> >> Eli, how did you reach that conclusion?
> > 
> > By adding the percentage of all the functions involved in decoding
> > strings.
> 
> Was the total sum of the profile 100%?

Yes.  Wasn't it 100% in your profiles?  I think it was.

> > I only accounted for time used by Emacs code, i.e. 100% doesn't
> > include the library.
> 
> I still don't understand. I my profiling report, decode_coding_utf_8 is 
> at the top (Emacs function). Followed by malloc and produce_chars. Are 
> you saying they are not involved in decoding strings?

Part (roughly half) of produce_chars is, malloc mostly isn't.

> > In any case, even if decoding takes 50% of the time we spend in Emacs
> > code, it is still not significant enough to justify the un-safety of
> > using a string that we didn't decode, because if that string ever
> > includes raw bytes, Emacs will surely crash.
> 
> What was the point of us doing the exercise, and me wasting time on it, 
> if you're set on not changing anything at all?

If the percentage of the decoding was much higher, like 80% or 90%,
say, then we would have a much better incentive to try to salvage at
least some of that, than we have now.

> We've already had the performance numbers from before.

The percentage of time taken by decoding wasn't clear to me, so I
wanted to profile.  Mind you, I intended to do that myself, I didn't
ask anyone to do it for me, although I'm of course grateful for your
work on this, as it confirmed my results.

> Given the encouragement, I really expected you to choose *some* path 
> toward improvement. Either forgoing conversion (maybe adding an 
> additional test suite, for cases you're worrying about), or doing some 
> kind of faster validation instead of full conversion (where we don't 
> allocate new strings but just check existing ones for validity), or, you 
> know, optimizing everything everywhere.

If someone has ideas how to speed up decode_coding_utf_8, please speak
up, and let's try that.  AFAICS, it is already heavily optimized for
the case of plain ASCII text, which is what we used in our profiling.
If plain ASCII text is important enough, perhaps we could make this
even faster, at the price of making non-ASCII cases slightly slower,
by adding a function that just validates plain ASCII without producing
a decoded string.

The original bug report found that we do unnecessary stuff like
allocating temporary buffers and calling various hook functions -- all
that is now gone, so we definitely have sped this up, probably
significantly in some non-"emacs -Q" cases.

> Considering libjansson manages to do both JSON parsing and string 
> conversion in ~the same time make_specified_string only does string 
> conversion on the returned strings, it most likely follows that 
> make_specified_string could be made faster.

FWIW, I have stream_get.part.3 in my profiles that takes almost twice
the time as decode_coding_utf_8, and various lex_* and other functions
also quite high on the profile, with 1.5% to 3.5%.  So it isn't like
libjansson is tenfold faster than our code.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: Sébastien Chapuis <sebastien@chapu.is>,
>  31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 22 Apr 2019 01:14:10 +0300
> 
> On 21.04.2019 13:23, yyoncho wrote:
> > IMO if validation of the data that comes from external libraries is 
> > taking a lot of time then it does not make sense to use external 
> > libraries at all when we are aiming for performance gains.
> 
> If we'll have to call make_specified_string for each parsed JSON string, 

What could/would be an alternative to that?  We must make a Lisp
string out of each string returned by libjansson.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: sebastien@chapu.is, 31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 22 Apr 2019 01:17:09 +0300
> 
> On 21.04.2019 15:15, Eli Zaretskii wrote:
> 
> > Then maybe we should look for a faster JSON parsing library, because
> > libjansson functions appear in the profile as high or higher than our
> > decoding routines.
> 
> Not in my profile.

No?  I have these near the top in mine:

  +   7.73%  emacs.jansson  emacs.jansson      [.] stream_get.part.3
  +   3.61%  emacs.jansson  emacs.jansson      [.] strbuffer_append_bytes
  +   3.47%  emacs.jansson  emacs.jansson      [.] lex_scan
  +   1.75%  emacs.jansson  emacs.jansson      [.] lex_get_save
  +   1.58%  emacs.jansson  emacs.jansson      [.] string_get

AFAICT, they are all from libjansson.  In this particular profile,
decode_coding_utf_8 looks like this:

  +   4.45%  emacs.jansson  emacs.jansson      [.] decode_coding_utf_8

> Again, if make_specified_string takes 50% of the runtime, even if 
> libjansson (or some other JSON lib) worked instantly, we'd only get a 2x 
> improvement at most.

I'm not sure it's 50%, as in my profile I have only this:

  +   1.41%  emacs.jansson  emacs.jansson      [.] make_uninit_multibyte_string

Please don't forget that json-parse-string first _encodes_ the input
string in UTF-8, then passes it to libjansson, then _decodes_ each
string element it receives back.  So we do some non-trivial processing
besides make_specified_string, and removing decoding only saves part
of that.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Mon, 22 Apr 2019 08:38:58 +0300
> Cc: Dmitry Gutov <dgutov@yandex.ru>, Sébastien Chapuis <sebastien@chapu.is>, 
> 	31138@debbugs.gnu.org
> 
>  read-from-string doesn't decode.  You need to time
>  decode-coding-string when the string is the entire JSON.
> 
> Can you elaborate? Both json-parse-string and read-from-string do the same from my point of view ->
> convert string to elisp data structure with read-from-string being 3times faster.

Maybe I misunderstood what you were trying to say.

If you were suggesting that we implement reading JSON objects as part
of Emacs, then yes, this won't need to encode/decode anything.  I'm
not sure the result will be as fast as read-from-string, since we are
talking about a different syntax.  Also, I'm not sure how hard would
that be nor how much maintenance will it require (does the JSON spec
change frequently?).  But if someone knows the answers, perhaps it's
an alternative worth considering.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 22.04.2019 1:12, Dmitry Gutov wrote:
> Considering libjansson manages to do both JSON parsing and string 
> conversion in ~the same time make_specified_string only does string 
> conversion on the returned strings, it most likely follows that 
> make_specified_string could be made faster.

^ Sorry, I meant code_convert_string here.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 22.04.2019 10:03, Eli Zaretskii wrote:

> Yes.  Wasn't it 100% in your profiles?  I think it was.

Err, it probably is. The above-1% percentages sum up to 54% percent 
total. There's a long tail, though.

>> I still don't understand. I my profiling report, decode_coding_utf_8 is
>> at the top (Emacs function). Followed by malloc and produce_chars. Are
>> you saying they are not involved in decoding strings?
> 
> Part (roughly half) of produce_chars is, malloc mostly isn't.

Okay, maybe some of it isn't part of code_convert_string, but libjansson 
functions themselves are way down below in my profile (though they 
probably also use malloc).

> If the percentage of the decoding was much higher, like 80% or 90%,
> say, then we would have a much better incentive to try to salvage at
> least some of that, than we have now.

As it is, we have an upper bound for optimizations: even with 
lightning-fast JSON library, performance will go up by 2x at most.

And if like you're saying in another email, we first encode the string 
before passing it to libjansson, then decode the strings in the result, 
we might be spending 70-80% of the time just doing encoding and decoding 
of strings in Emacs, if, say, encoding is twice as fast as decoding.

>> We've already had the performance numbers from before.
> 
> The percentage of time taken by decoding wasn't clear to me, so I
> wanted to profile.  Mind you, I intended to do that myself, I didn't
> ask anyone to do it for me, although I'm of course grateful for your
> work on this, as it confirmed my results.

My profiling report was very different. The conclusion was, too.

> If someone has ideas how to speed up decode_coding_utf_8, please speak
> up, and let's try that.  AFAICS, it is already heavily optimized for
> the case of plain ASCII text, which is what we used in our profiling.
> If plain ASCII text is important enough, perhaps we could make this
> even faster, at the price of making non-ASCII cases slightly slower,
> by adding a function that just validates plain ASCII without producing
> a decoded string.

I'm going to have to leave that up to somebody more well-versed with C 
and Unicode.

Speaking of the specific proposal, I did something like the latter with 
json.el in the past (speeding up the ASCII-only case). It was a 
significant win.

> The original bug report found that we do unnecessary stuff like
> allocating temporary buffers and calling various hook functions -- all
> that is now gone, so we definitely have sped this up, probably
> significantly in some non-"emacs -Q" cases.

That is true. Thank you.

> FWIW, I have stream_get.part.3 in my profiles that takes almost twice
> the time as decode_coding_utf_8, and various lex_* and other functions
> also quite high on the profile, with 1.5% to 3.5%.  So it isn't like
> libjansson is tenfold faster than our code.

I don't see that in mine.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 21.04.2019 16:33, Philipp Stephani wrote:
> Am So., 21. Apr. 2019 um 15:10 Uhr schrieb yyoncho<yyoncho@gmail.com>:
>> Hi Philipp,
>>
>> As Eli commented in this thread, the decoding is must have for all external libraries no matter what is their contract.
> I disagree.

If I simply apply the patch from 
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=31138#5, that breaks tests 
in test/src/json-tests.el. In particular, json-parse-string/string:

F json-parse-string/string
     (ert-test-failed
      ((should
        (equal
         (json-parse-string "[\"abcαβγ\"]")
         ["abcαβγ"]))
       :form
       (equal
        ["abc\316\261\316\262\316\263"]
        ["abcαβγ"])
       :value nil :explanation
       (array-elt 0
                  (arrays-of-different-length 9 6 
"abc\316\261\316\262\316\263" "abcαβγ" first-mismatch-at 3))))

Would somebody like to propose an alternative patch?

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Cc: sebastien@chapu.is, yyoncho@gmail.com, 31138@debbugs.gnu.org
> Date: Mon, 22 Apr 2019 14:36:02 +0300
> 
> On 22.04.2019 1:12, Dmitry Gutov wrote:
> > Considering libjansson manages to do both JSON parsing and string 
> > conversion in ~the same time make_specified_string only does string 
> > conversion on the returned strings, it most likely follows that 
> > make_specified_string could be made faster.
> 
> ^ Sorry, I meant code_convert_string here.

That one is called both for encoding the input and for decoding the
strings in the result of parsing.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: sebastien@chapu.is, yyoncho@gmail.com, 31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 22 Apr 2019 14:46:20 +0300
> 
> > FWIW, I have stream_get.part.3 in my profiles that takes almost twice
> > the time as decode_coding_utf_8, and various lex_* and other functions
> > also quite high on the profile, with 1.5% to 3.5%.  So it isn't like
> > libjansson is tenfold faster than our code.
> 
> I don't see that in mine.

I don't know why.  In my profiles I enlarged the iteration count to
100, and also enlarged the sampling frequency.  Maybe that's the
reason.  The percentage taken by decode_coding_utf_8 didn't change as
result, FWIW.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: Eli Zaretskii <eliz@gnu.org>, Sébastien Chapuis
>  <sebastien@chapu.is>, 31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 22 Apr 2019 14:48:52 +0300
> 
> If I simply apply the patch from 
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=31138#5, that breaks tests 
> in test/src/json-tests.el. In particular, json-parse-string/string:
> 
> F json-parse-string/string
>      (ert-test-failed
>       ((should
>         (equal
>          (json-parse-string "[\"abcαβγ\"]")
>          ["abcαβγ"]))
>        :form
>        (equal
>         ["abc\316\261\316\262\316\263"]
>         ["abcαβγ"])
>        :value nil :explanation
>        (array-elt 0
>                   (arrays-of-different-length 9 6 
> "abc\316\261\316\262\316\263" "abcαβγ" first-mismatch-at 3))))

That patch causes us to create unibyte strings, so it's small wonder
you see these failures.

> Would somebody like to propose an alternative patch?

Try using 'true' as the last argument of make_specified_string.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 22.04.2019 15:12, Eli Zaretskii wrote:

> Try using 'true' as the last argument of make_specified_string.

Thank you. Tried it, tests now pass, and the performance improvement is 
the same. I compared the same benchmark (100 iterations, GC disabled for 
the whole duration), and this patch takes

src/emacs -Q --batch -l ~/examples/elisp/json-test.el
Elapsed time: 51.153870s

down to

$ src/emacs -Q --batch -l ~/examples/elisp/json-test.el
Elapsed time: 26.268435s

Are you still against it? (Just checking).

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 22.04.2019 15:07, Eli Zaretskii wrote:
> I don't know why.  In my profiles I enlarged the iteration count to
> 100, and also enlarged the sampling frequency.  Maybe that's the
> reason.

I posted my invocations here, with all arguments. I didn't see yours.

> The percentage taken by decode_coding_utf_8 didn't change as
> result, FWIW.

Maybe it's not the only bottleneck. What about produce_chars? And 
malloc, of course. Here's the top of my profiling report again:

  6.00%   229  emacs    emacs                 [.] decode_coding_utf_8
  5.31%   206  emacs    libc-2.27.so          [.] malloc
  4.57%   177  emacs    emacs                 [.] produce_chars
  4.12%   164  emacs    libc-2.27.so          [.] malloc_consolidate
  4.05%   161  emacs    libc-2.27.so          [.] cfree@GLIBC_2.2.5
  3.38%   135  emacs    libc-2.27.so    [.] __memmove_avx_unaligned_erms
  3.23%   125  emacs    emacs                 [.] store_symval_forwarding
  2.63%   102  emacs    emacs                 [.] do_symval_forwarding
  2.58%   104  emacs    libc-2.27.so          [.] _int_malloc
  2.58%   100  emacs    emacs                 [.] set_buffer_internal_2
  2.41%    93  emacs    emacs                 [.] allocate_vectorlike
  2.08%    81  emacs    emacs                 [.] json_to_lisp
  1.90%    81  emacs    emacs                 [.] detect_coding_utf_8
  1.85%    72  emacs    emacs           [.] swap_in_symval_forwarding
  1.72%    72  emacs    emacs                 [.] detect_coding
  1.38%    31  emacs    emacs                 [.] insert_from_gap
  1.29%    50  emacs    libjansson.so.4.11.0  [.] json_delete
  1.19%    46  emacs    emacs                 [.] hash_lookup
  1.01%    39  emacs    emacs                 [.] decode_coding
  1.01%    39  emacs    libjansson.so.4.11.0  [.] json_array_get

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 632 bytes --]

Hi Eli,

If you were suggesting that we implement reading JSON objects as part
> of Emacs, then yes, this won't need to encode/decode anything.  I'm
> not sure the result will be as fast as read-from-string, since we are
> talking about a different syntax.  Also, I'm not sure how hard would
> that be nor how much maintenance will it require (does the JSON spec
> change frequently?).  But if someone knows the answers, perhaps it's
> an alternative worth considering.
>

JSON spec is stable. FYI json.el parser is 568 loc. Both formats have a
similar complexity with json being a bit simpler(fewer data structures).

Thanks,
Ivan

[-- Attachment #2: Type: text/html, Size: 962 bytes --]

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: p.stephani2@gmail.com, sebastien@chapu.is, yyoncho@gmail.com,
>  31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 22 Apr 2019 15:24:24 +0300
> 
> Thank you. Tried it, tests now pass, and the performance improvement is 
> the same. I compared the same benchmark (100 iterations, GC disabled for 
> the whole duration), and this patch takes
> 
> src/emacs -Q --batch -l ~/examples/elisp/json-test.el
> Elapsed time: 51.153870s
> 
> down to
> 
> $ src/emacs -Q --batch -l ~/examples/elisp/json-test.el
> Elapsed time: 26.268435s
> 
> Are you still against it? (Just checking).

I'm still against using that patch as is, yes.  I'm okay with using
make_specified_string if before calling it we make sure the string is
plain ASCII or a series of proper UTF-8 sequences.  Not sure how much
of a performance hit would such tests cost us, but if you are
interested, let's time them.

(Let me know if you need help in writing the code for the above 2
tests.  I think parse_str_as_multibyte should help a lot.)

I guess we should also have some test case with non-ASCII characters,
if we will introduce these optimizations.

Thanks.

P.S.  I'm still not sure these optimizations will make the OP happy,
since at some point I heard them saying that our present performance
is abysmally slow and inadequate.  If that wasn't a wild exaggeration,
then halving the time will still be inadequate.  So maybe we should
agree in advance whether 30% to 50% improvement will be "good enough",
before we embark on this adventure.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 22.04.2019 15:01, Eli Zaretskii wrote:

>> On 22.04.2019 1:12, Dmitry Gutov wrote:
>>> Considering libjansson manages to do both JSON parsing and string
>>> conversion in ~the same time make_specified_string only does string
>>> conversion on the returned strings, it most likely follows that
>>> make_specified_string could be made faster.
>>
>> ^ Sorry, I meant code_convert_string here.
> 
> That one is called both for encoding the input and for decoding the
> strings in the result of parsing.

Right. So only the call to code_convert_string from json_make_string. 
The decoding part.

So roughly:

String encoding plus libjansson parsing plus other logic in json.c takes 
~the same time as string decoding only.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: sebastien@chapu.is, yyoncho@gmail.com, 31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 22 Apr 2019 15:58:57 +0300
> 
> On 22.04.2019 15:07, Eli Zaretskii wrote:
> > I don't know why.  In my profiles I enlarged the iteration count to
> > 100, and also enlarged the sampling frequency.  Maybe that's the
> > reason.
> 
> I posted my invocations here, with all arguments. I didn't see yours.

They are the same.  The difference is that you visit the file inside
the benchmark loop, whereas I left only the conversion inside the
loop.  AFAIU, doing so produces a better measurement of the JSON
parsing part alone.

> > The percentage taken by decode_coding_utf_8 didn't change as
> > result, FWIW.
> 
> Maybe it's not the only bottleneck. What about produce_chars? And 
> malloc, of course. Here's the top of my profiling report again:

produce_chars is a subroutine of any decoding routine, including
decode_coding_utf_8, so I added that to the score.  In my profile,
produce_chars took 3.77%.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Mon, 22 Apr 2019 16:00:13 +0300
> Cc: Dmitry Gutov <dgutov@yandex.ru>, Sébastien Chapuis <sebastien@chapu.is>, 
> 	31138@debbugs.gnu.org
> 
> JSON spec is stable. FYI json.el parser is 568 loc. Both formats have a similar complexity with json being a
> bit simpler(fewer data structures).

Then I hope someone will try implementing that as part of Emacs, and
we will see the resulting performance.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 22.04.2019 10:16, Eli Zaretskii wrote:
> Please don't forget that json-parse-string first_encodes_  the input
> string in UTF-8, then passes it to libjansson, then_decodes_  each
> string element it receives back.  So we do some non-trivial processing
> besides make_specified_string, and removing decoding only saves part
> of that.

That actually means that speeding up code_convert_string might 
*potentially* (e.g. if it's infinitely fast) yield more than 50% 
performance improvement.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 22.04.2019 16:12, Eli Zaretskii wrote:
> They are the same.  The difference is that you visit the file inside
> the benchmark loop, whereas I left only the conversion inside the
> loop.

Interesting. I'll try that out as well sometime.

 > AFAIU, doing so produces a better measurement of the JSON
 > parsing part alone.

But not a better measurement of how json.c is actually used, I'm afraid 
(unless you're trying to cut out the time Emacs spends reading from 
disk). So even if my benchmark gives more attention to buffer-decoding 
routines, it probably reflects practical usage too (JSON usually comes 
from HTTP response, and it has to be decoded as well).

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 22.04.2019 16:02, Eli Zaretskii wrote:

>> Thank you. Tried it, tests now pass, and the performance improvement is
>> the same. I compared the same benchmark (100 iterations, GC disabled for
>> the whole duration), and this patch takes
>>
>> src/emacs -Q --batch -l ~/examples/elisp/json-test.el
>> Elapsed time: 51.153870s
>>
>> down to
>>
>> $ src/emacs -Q --batch -l ~/examples/elisp/json-test.el
>> Elapsed time: 26.268435s
>>
>> Are you still against it? (Just checking).
> 
> I'm still against using that patch as is, yes.  I'm okay with using
> make_specified_string if before calling it we make sure the string is
> plain ASCII or a series of proper UTF-8 sequences.  Not sure how much
> of a performance hit would such tests cost us, but if you are
> interested, let's time them.

Sure.

> (Let me know if you need help in writing the code for the above 2
> tests.  I think parse_str_as_multibyte should help a lot.)

I do.

At the very least: am I supposed to use parse_str_as_multibyte similarly 
to how make_string does, or to write a function similar to 
parse_str_as_multibyte? I can more or less follow its logic, but I don't 
understand if any of its callees cannot cope with improper input.

> I guess we should also have some test case with non-ASCII characters,
> if we will introduce these optimizations.

We already do in test/src/json-tests.el, like I previously mentioned. 
And the simple patch (which you're against) passes them. I've put the 
patch at the end of this email so we're on the same page.

> Thanks.
> 
> P.S.  I'm still not sure these optimizations will make the OP happy,
> since at some point I heard them saying that our present performance
> is abysmally slow and inadequate.

Well... IIUC Node.js's JSON parsing is ~10 times as fast. Ruby's parser 
speeds vary from ~9 times as fast to ~3 as fast, for comparison.

For LSP usage, we are of course comparing with Node. But since we're 
still here, and lsp-mode has some users, reaching Node's performance 
level is likely not a life-or-death situation.

> If that wasn't a wild exaggeration,
> then halving the time will still be inadequate.  So maybe we should
> agree in advance whether 30% to 50% improvement will be "good enough",
> before we embark on this adventure.

If we're talking about big changes and increases in complexity, sure, we 
should weigh them. But if a simple change gives us even a 20-30% 
improvement, why not take it? The reporter is not the only one who 
parses JSON in Emacs.

Speaking of bigger improvements... it seems that with the patch below, 
and the fact that it passes the existing tests, we have at least 
established that the contents of the C strings that libjansson returns 
and our "decoded" strings are very often exactly the same. So most of 
the time what code_convert_string does is not really conversion, but in 
effect verification. I'm betting it's a frequent situation in other use 
cases, too.

So one optimization (more complex to implement, I'm sure) would be to 
defer creating coding->dst_object inside decode_coding_object until 
we're sure we need it (the source and destination bytes actually come 
out different), and if we don't, return src_object in the end (I'm only 
taking about the case when dst_object is Qt). That might improve 
performance across the board, including during the encoding step. Or 
might not, of course. What do you think?

diff --git a/src/json.c b/src/json.c
index 928825e034..2b0cc8a313 100644
--- a/src/json.c
+++ b/src/json.c
@@ -225,8 +225,7 @@ json_has_suffix (const char *string, const char *suffix)
  static Lisp_Object
  json_make_string (const char *data, ptrdiff_t size)
  {
-  return code_convert_string (make_specified_string (data, -1, size, 
false),
-                              Qutf_8_unix, Qt, false, true, true);
+  return make_specified_string (data, -1, size, false);
  }

  /* Create a multibyte Lisp string from the NUL-terminated UTF-8

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: yyoncho@gmail.com, sebastien@chapu.is, 31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 22 Apr 2019 16:54:19 +0300
> 
> On 22.04.2019 10:16, Eli Zaretskii wrote:
> > Please don't forget that json-parse-string first_encodes_  the input
> > string in UTF-8, then passes it to libjansson, then_decodes_  each
> > string element it receives back.  So we do some non-trivial processing
> > besides make_specified_string, and removing decoding only saves part
> > of that.
> 
> That actually means that speeding up code_convert_string might 
> *potentially* (e.g. if it's infinitely fast) yield more than 50% 
> performance improvement.

code_convert_string doesn't do much, it just calls the encoding or
decoding function.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: sebastien@chapu.is, yyoncho@gmail.com, 31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 22 Apr 2019 16:58:05 +0300
> 
>  > AFAIU, doing so produces a better measurement of the JSON
>  > parsing part alone.
> 
> But not a better measurement of how json.c is actually used, I'm afraid 
> (unless you're trying to cut out the time Emacs spends reading from 
> disk). So even if my benchmark gives more attention to buffer-decoding 
> routines, it probably reflects practical usage too (JSON usually comes 
> from HTTP response, and it has to be decoded as well).

But it shows the upper limit of what we can expect, and it avoids the
danger of having unrelated factors obscuring the actual performance.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 22.04.2019 18:24, Eli Zaretskii wrote:
>> That actually means that speeding up code_convert_string might
>> *potentially*  (e.g. if it's infinitely fast) yield more than 50%
>> performance improvement.
> code_convert_string doesn't do much, it just calls the encoding or
> decoding function.

Right. I don't mean that the change would be inside 
code_convert_string's implementation necessarily. Probably in some of 
functions it calls.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: p.stephani2@gmail.com, sebastien@chapu.is, yyoncho@gmail.com,
>  31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 22 Apr 2019 18:02:35 +0300
> 
> > (Let me know if you need help in writing the code for the above 2
> > tests.  I think parse_str_as_multibyte should help a lot.)
> 
> I do.
> 
> At the very least: am I supposed to use parse_str_as_multibyte similarly 
> to how make_string does, or to write a function similar to 
> parse_str_as_multibyte? I can more or less follow its logic, but I don't 
> understand if any of its callees cannot cope with improper input.

Let's start with just ASCII strings, and then consider moving to valid
UTF-8 sequences.  I take it you can easily write a loop that ensures a
string is pure ASCII?

> > I guess we should also have some test case with non-ASCII characters,
> > if we will introduce these optimizations.
> 
> We already do in test/src/json-tests.el, like I previously mentioned. 

No, I meant a test of performance.  if we begin by testing for plain
ASCII strings, then non-ASCII strings will take longer to convert.
The existing tests are too short to support measurement of the effect,
we need a larger JSON object with many non-ASCII strings.

> If we're talking about big changes and increases in complexity, sure, we 
> should weigh them. But if a simple change gives us even a 20-30% 
> improvement, why not take it? The reporter is not the only one who 
> parses JSON in Emacs.

Suit yourself, but I don't like investing hours in code just to hear
"your best is not good enough" from those who triggered the changes to
begin with.

> Speaking of bigger improvements... it seems that with the patch below, 
> and the fact that it passes the existing tests, we have at least 
> established that the contents of the C strings that libjansson returns 
> and our "decoded" strings are very often exactly the same. So most of 
> the time what code_convert_string does is not really conversion, but in 
> effect verification. I'm betting it's a frequent situation in other use 
> cases, too.

Of course.  Verification code almost always comes up empty-handed.  it
doesn't mean we can throw it away.

> So one optimization (more complex to implement, I'm sure) would be to 
> defer creating coding->dst_object inside decode_coding_object until 
> we're sure we need it (the source and destination bytes actually come 
> out different), and if we don't, return src_object in the end (I'm only 
> taking about the case when dst_object is Qt). That might improve 
> performance across the board, including during the encoding step. Or 
> might not, of course. What do you think?

I don't want to make changes that affect decoding everywhere, because
having raw bytes in other cases is a more frequent phenomenon.  Let's
just optimize JSON parsing, OK?

> 
> diff --git a/src/json.c b/src/json.c
> index 928825e034..2b0cc8a313 100644
> --- a/src/json.c
> +++ b/src/json.c
> @@ -225,8 +225,7 @@ json_has_suffix (const char *string, const char *suffix)
>   static Lisp_Object
>   json_make_string (const char *data, ptrdiff_t size)
>   {
> -  return code_convert_string (make_specified_string (data, -1, size, 
> false),
> -                              Qutf_8_unix, Qt, false, true, true);
> +  return make_specified_string (data, -1, size, false);
                                                   ^^^^^
Should be 'true', right?

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 22.04.2019 18:25, Eli Zaretskii wrote:

>> But not a better measurement of how json.c is actually used, I'm afraid
>> (unless you're trying to cut out the time Emacs spends reading from
>> disk). So even if my benchmark gives more attention to buffer-decoding
>> routines, it probably reflects practical usage too (JSON usually comes
>> from HTTP response, and it has to be decoded as well).
> 
> But it shows the upper limit of what we can expect,
Not if we manage to optimize the commonly-used subroutines as well.

insert-file-contents probably uses code_convert_string somewhere inside, 
right?

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: sebastien@chapu.is, yyoncho@gmail.com, 31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 22 Apr 2019 18:41:00 +0300
> 
> insert-file-contents probably uses code_convert_string somewhere inside, 
> right?

Yes, but that's irrelevant.  (You should have used
insert-file-contents-literally instead.)  And I don't think we will be
able to speed up the general-purpose decoding routines: no one said
the original file is encoded in UTF-8.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 22.04.2019 18:50, Eli Zaretskii wrote:
> (You should have used
> insert-file-contents-literally instead.)

I wonder if there's a counterpart for that for HTTP responses returned 
by url-retrieve.

> And I don't think we will be
> able to speed up the general-purpose decoding routines: no one said
> the original file is encoded in UTF-8.

We could speed up certain important cases, like where the bytes are 
identical between the source and the destination. I could be wrong here, 
though, this is already out of my depth.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 1177 bytes --]

On 22.04.2019 18:36, Eli Zaretskii wrote:
> Let's start with just ASCII strings, and then consider moving to valid
> UTF-8 sequences.  I take it you can easily write a loop that ensures a
> string is pure ASCII?

All right. Does the attached json_encode_string_ascii_test.diff look 
good to you? In terms of correctness/safety, I mean.

> No, I meant a test of performance.  if we begin by testing for plain
> ASCII strings, then non-ASCII strings will take longer to convert.
> The existing tests are too short to support measurement of the effect,
> we need a larger JSON object with many non-ASCII strings.

Makes sense.

> Suit yourself, but I don't like investing hours in code just to hear
> "your best is not good enough" from those who triggered the changes to
> begin with.

I have code that parses JSON as well. Mentioned that before.

> I don't want to make changes that affect decoding everywhere, because
> having raw bytes in other cases is a more frequent phenomenon.  Let's
> just optimize JSON parsing, OK?
> 
> Should be 'true', right?

Erm, right. >_< Probably hit 'undo' one too many times.

I've attached this patch as json_make_string_no_validation.diff.

[-- Attachment #2: json_make_string_ascii_test.diff --]
[-- Type: text/x-patch, Size: 1019 bytes --]

diff --git a/src/json.c b/src/json.c
index 928825e034..033fa27638 100644
--- a/src/json.c
+++ b/src/json.c
@@ -225,8 +225,17 @@ json_has_suffix (const char *string, const char *suffix)
 static Lisp_Object
 json_make_string (const char *data, ptrdiff_t size)
 {
-  return code_convert_string (make_specified_string (data, -1, size, false),
-                              Qutf_8_unix, Qt, false, true, true);
+  ptrdiff_t nchars, multibyte_nbytes;
+  parse_str_as_multibyte ((const unsigned char *) data, size,
+                          &nchars, &multibyte_nbytes);
+
+  if (size == nchars)
+    /* Wasn't sure if the last arg should better be true here, but
+       it doesn't seem to affect the behavior or correctness, so far. */
+    return make_specified_string (data, -1, size, false);
+  else
+    return code_convert_string (make_specified_string (data, -1, size, false),
+                                Qutf_8_unix, Qt, false, true, true);
 }
 
 /* Create a multibyte Lisp string from the NUL-terminated UTF-8

[-- Attachment #3: json_make_string_no_validation.diff --]
[-- Type: text/x-patch, Size: 532 bytes --]

diff --git a/src/json.c b/src/json.c
index 928825e034..b06d411032 100644
--- a/src/json.c
+++ b/src/json.c
@@ -225,8 +225,7 @@ json_has_suffix (const char *string, const char *suffix)
 static Lisp_Object
 json_make_string (const char *data, ptrdiff_t size)
 {
-  return code_convert_string (make_specified_string (data, -1, size, false),
-                              Qutf_8_unix, Qt, false, true, true);
+  return make_specified_string (data, -1, size, true);
 }
 
 /* Create a multibyte Lisp string from the NUL-terminated UTF-8

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: sebastien@chapu.is, yyoncho@gmail.com, 31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 22 Apr 2019 19:00:13 +0300
> 
> On 22.04.2019 18:50, Eli Zaretskii wrote:
> > (You should have used
> > insert-file-contents-literally instead.)

(And, to answer your question: insert-file-contents doesn't use
code_convert_string, it decodes text as it being read, directly in the
buffer's gap.)

> I wonder if there's a counterpart for that for HTTP responses returned 
> by url-retrieve.

There is, if we need: bind coding-system-for-read to 'no-conversion'.

> > And I don't think we will be
> > able to speed up the general-purpose decoding routines: no one said
> > the original file is encoded in UTF-8.
> 
> We could speed up certain important cases, like where the bytes are 
> identical between the source and the destination.

Not sure what you mean by "source" and "destination".

In any case, coding.c is already heavily optimized, certainly in the
UTF-8 case.  You and others are welcome to profile the code looking
for opportunities to optimize even more, but I'd be surprised if you
find something significant.  In particular, UTF-8 decoding validates
as part of the decoding loop, and is already optimized for ASCII, so
it will be interesting to see what kind of gains we could have in
json_make_string by testing for ASCII up front.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: p.stephani2@gmail.com, sebastien@chapu.is, yyoncho@gmail.com,
>  31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 22 Apr 2019 19:16:15 +0300
> 
> All right. Does the attached json_encode_string_ascii_test.diff look 
> good to you?

The size == nchars case not necessarily true just for ASCII, I think,
and the code in parse_str_as_multibyte is more complex than needed
just for ASCII.  I meant make a loop that literally checks only for
ASCII_CHAR_P, and nothing more.  I think it makes sense to see first
how much can we speed up things for pure ASCII JSONs.

> +  if (size == nchars)
> +    /* Wasn't sure if the last arg should better be true here, but
> +       it doesn't seem to affect the behavior or correctness, so far. */
> +    return make_specified_string (data, -1, size, false);

It should make no difference for ASCII, but I think I prefer to use
'true' there, for consistency with the other cases.

Thanks.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 22.04.2019 19:28, Eli Zaretskii wrote:
>> Cc: p.stephani2@gmail.com, sebastien@chapu.is, yyoncho@gmail.com,
>>   31138@debbugs.gnu.org
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Date: Mon, 22 Apr 2019 19:16:15 +0300
>>
>> All right. Does the attached json_encode_string_ascii_test.diff look
>> good to you?
> 
> The size == nchars case not necessarily true just for ASCII, I think,
> and the code in parse_str_as_multibyte is more complex than needed
> just for ASCII.  I meant make a loop that literally checks only for
> ASCII_CHAR_P, and nothing more.  I think it makes sense to see first
> how much can we speed up things for pure ASCII JSONs.

OK, I'll try to do what you've described here.

In the meantime, I'd like to report that the aforementioned patch, even 
though too-complex and not entirely correct, provides practically all 
the performance improvement we get from the no-validation patch.

IOW, with my test scenario:

current master:  51s
no_validation:   21s
ascii_test:      22s

>> +  if (size == nchars)
>> +    /* Wasn't sure if the last arg should better be true here, but
>> +       it doesn't seem to affect the behavior or correctness, so far. */
>> +    return make_specified_string (data, -1, size, false);
> 
> It should make no difference for ASCII, but I think I prefer to use
> 'true' there, for consistency with the other cases.

I left 'false' there for testing, to make sure we hit both branches of 
the condition (the tests still pass).

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: p.stephani2@gmail.com, sebastien@chapu.is, yyoncho@gmail.com,
>  31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 22 Apr 2019 19:16:15 +0300
> 
> +  if (size == nchars)
> +    /* Wasn't sure if the last arg should better be true here, but
> +       it doesn't seem to affect the behavior or correctness, so far. */
> +    return make_specified_string (data, -1, size, false);
> +  else
> +    return code_convert_string (make_specified_string (data, -1, size, false),
> +                                Qutf_8_unix, Qt, false, true, true);

Btw, we could somewhat speed up even the case where we do call
code_convert_string, by avoiding the call to make_specified_string.
That's because decode_coding_object, called by code_convert_string to
perform the decoding, is capable of decoding a C string, if we set up
its argument accordingly.  You can see one example of that in
w32select.c:w32-get-clipboard-data.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: p.stephani2@gmail.com, sebastien@chapu.is, yyoncho@gmail.com,
>  31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 22 Apr 2019 19:44:18 +0300
> 
> In the meantime, I'd like to report that the aforementioned patch, even 
> though too-complex and not entirely correct, provides practically all 
> the performance improvement we get from the no-validation patch.
> 
> IOW, with my test scenario:
> 
> current master:  51s
> no_validation:   21s
> ascii_test:      22s

That's good to hear, thanks.

bug#31138: Native json slower than json.el

[Ivan]

Hi Eli, Dmitry,


Do you know whether this library was considered when implementing the 
json parser?

https://github.com/zserge/jsmn

Based on my limited understanding I think that it might be better fit 
for what we are trying to solve.


Thanks,

Ivan



On 4/22/19 4:17 PM, Eli Zaretskii wrote:
>> From: yyoncho <yyoncho@gmail.com>
>> Date: Mon, 22 Apr 2019 16:00:13 +0300
>> Cc: Dmitry Gutov <dgutov@yandex.ru>, Sébastien Chapuis <sebastien@chapu.is>,
>> 	31138@debbugs.gnu.org
>>
>> JSON spec is stable. FYI json.el parser is 568 loc. Both formats have a similar complexity with json being a
>> bit simpler(fewer data structures).
> Then I hope someone will try implementing that as part of Emacs, and
> we will see the resulting performance.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: sebastien@chapu.is, 31138@debbugs.gnu.org
> From: Ivan <yyoncho@gmail.com>
> Date: Mon, 22 Apr 2019 19:53:05 +0300
> 
> Do you know whether this library was considered when implementing the 
> json parser?
> 
> https://github.com/zserge/jsmn

See http://lists.gnu.org/archive/html/emacs-devel/2017-09/msg00528.html.
Looks like the answer is yes.

> Based on my limited understanding I think that it might be better fit 
> for what we are trying to solve.

Feel free to tell why on emacs-devel.

Thanks.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 320 bytes --]

On 22.04.2019 19:28, Eli Zaretskii wrote:
> I meant make a loop that literally checks only for
> ASCII_CHAR_P, and nothing more.

Is this (attached) what you had in mind?

It's a bit faster again (naturally). At this point I don't see any 
statistically significant performance difference from the no_validation 
patch.

[-- Attachment #2: json_make_string_ascii_test_v2.diff --]
[-- Type: text/x-patch, Size: 1055 bytes --]

diff --git a/src/json.c b/src/json.c
index 928825e034..9c03d09ca6 100644
--- a/src/json.c
+++ b/src/json.c
@@ -225,8 +225,25 @@ json_has_suffix (const char *string, const char *suffix)
 static Lisp_Object
 json_make_string (const char *data, ptrdiff_t size)
 {
-  return code_convert_string (make_specified_string (data, -1, size, false),
-                              Qutf_8_unix, Qt, false, true, true);
+  char* ptr = (char*)data + size;
+  bool ascii_only = true;
+
+  while (ptr > data) {
+    if (!ASCII_CHAR_P(*ptr))
+      {
+        ascii_only = false;
+        break;
+      }
+    ptr--;
+  }
+
+  if (ascii_only)
+    /* Wasn't sure if the last arg should better be true here, but
+       it doesn't seem to affect the behavior or correctness, so far. */
+    return make_specified_string (data, -1, size, false);
+  else
+    return code_convert_string (make_specified_string (data, -1, size, false),
+                                Qutf_8_unix, Qt, false, true, true);
 }
 
 /* Create a multibyte Lisp string from the NUL-terminated UTF-8

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 22.04.2019 19:49, Eli Zaretskii wrote:

> Btw, we could somewhat speed up even the case where we do call
> code_convert_string, by avoiding the call to make_specified_string.
> That's because decode_coding_object, called by code_convert_string to
> perform the decoding, is capable of decoding a C string, if we set up
> its argument accordingly.

Guess we could test that with a non-ascii sample.

I was kind of expecting to continue on the "fast validation" route, and 
check whether DATA contains non-ASCII bytes in the exact way we want 
them. And simply abort otherwise.

> You can see one example of that in
> w32select.c:w32-get-clipboard-data.

Hmm, I'm not seeing any calls to code_convert_string in w32select.c.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Date: Mon, 22 Apr 2019 19:49:28 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: p.stephani2@gmail.com, sebastien@chapu.is, yyoncho@gmail.com,
> 	31138@debbugs.gnu.org
> 
> Btw, we could somewhat speed up even the case where we do call
> code_convert_string, by avoiding the call to make_specified_string.

Like this:

diff --git a/src/json.c b/src/json.c
index 16500bc..48aeccf 100644
--- a/src/json.c
+++ b/src/json.c
@@ -225,8 +225,12 @@ json_has_suffix (const char *string, const char *suffix)
 static Lisp_Object
 json_make_string (const char *data, ptrdiff_t size)
 {
-  return code_convert_string (make_specified_string (data, -1, size, false),
-                              Qutf_8_unix, Qt, false, true, true);
+  struct coding_system coding;
+  setup_coding_system (Qutf_8_unix, &coding);
+  coding.mode |= CODING_MODE_LAST_BLOCK;
+  coding.source = data;
+  decode_coding_object (&coding, Qnil, 0, 0, size, size, Qt);
+  return coding.dst_object;
 }
 
 /* Create a multibyte Lisp string from the NUL-terminated UTF-8


How does this fare in your benchmarks?

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: p.stephani2@gmail.com, sebastien@chapu.is, yyoncho@gmail.com,
>  31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 22 Apr 2019 20:05:46 +0300
> 
> > I meant make a loop that literally checks only for
> > ASCII_CHAR_P, and nothing more.
> 
> Is this (attached) what you had in mind?

Yes, but I'm slightly surprised why you loop from the end of the
string and not from the beginning.

> It's a bit faster again (naturally). At this point I don't see any 
> statistically significant performance difference from the no_validation 
> patch.

I guess that's expected when the strings in JSON are short enough.

The next question is what speedup you will see from the optimized
decoding version I just posted.  Then we could decide whether testing
for valid UTF-8 non-ASCII text is justified.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: p.stephani2@gmail.com, sebastien@chapu.is, yyoncho@gmail.com,
>  31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 22 Apr 2019 20:11:33 +0300
> 
> I was kind of expecting to continue on the "fast validation" route, and 
> check whether DATA contains non-ASCII bytes in the exact way we want 
> them. And simply abort otherwise.

I don't think we should abort, we don't do that anywhere else.

> > You can see one example of that in
> > w32select.c:w32-get-clipboard-data.
> 
> Hmm, I'm not seeing any calls to code_convert_string in w32select.c.

Of course!  Look for decode_coding_object.

bug#31138: Native json slower than json.el

[Alex Gramiak]

[-- Attachment #1: Type: text/plain, Size: 522 bytes --]

If the validation is done on the Emacs side, then perhaps it would be
beneficial to use the json_*_nocheck function alternatives to disable
libjannson's validation, namely json_stringn_nocheck and
json_object_set_new_nocheck.

[1] jansson.readthedocs.io/en/stable/apiref.html#c.json_stringn_nocheck

P.S. Would applying the following diff be okay? It's a simple
optimization that avoids an overflow check (the sizeof comparison is
optimized out even in -O0) in the usual case of a JSON integer fitting
into an EMACS_INT.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sizeof --]
[-- Type: text/x-patch, Size: 519 bytes --]

diff --git a/src/json.c b/src/json.c
index 928825e034..9faf1f80ca 100644
--- a/src/json.c
+++ b/src/json.c
@@ -836,7 +836,10 @@ json_to_lisp (json_t *json, struct json_configuration *conf)
     case JSON_INTEGER:
       {
 	json_int_t i = json_integer_value (json);
-	return INT_TO_INTEGER (i);
+        if (sizeof (json_int_t) <= sizeof (EMACS_INT))
+          return make_fixnum (i);
+        else
+          return INT_TO_INTEGER (i);
       }
     case JSON_REAL:
       return make_float (json_real_value (json));

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: Alex Gramiak <agrambot@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 22 Apr 2019 12:20:15 -0600
> 
> P.S. Would applying the following diff be okay? It's a simple
> optimization that avoids an overflow check (the sizeof comparison is
> optimized out even in -O0)

I don't understand what this optimizes, since AFAIU INT_TO_INTEGER
already produces a fixnum if possible.  It sounds like a step
backward, because the whole point of INT_TO_INTEGER is that the
programmer doesn't need to bother about the magnitude of the value.

> in the usual case of a JSON integer fitting into an EMACS_INT.

Not sure what you meant by "usual case".  Does this include a 32-bit
build of Emacs?

bug#31138: Native json slower than json.el

[Alex Gramiak]

[-- Attachment #1: Type: text/plain, Size: 811 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex Gramiak <agrambot@gmail.com>
>> Cc: Eli Zaretskii <eliz@gnu.org>, Dmitry Gutov <dgutov@yandex.ru>
>> Date: Mon, 22 Apr 2019 12:20:15 -0600
>> 
>> P.S. Would applying the following diff be okay? It's a simple
>> optimization that avoids an overflow check (the sizeof comparison is
>> optimized out even in -O0)
>
> I don't understand what this optimizes, since AFAIU INT_TO_INTEGER
> already produces a fixnum if possible.

I was attempting to avoid the FIXNUM_OVERFLOW_P check that make_(u)int
does, but I wasn't taking into consideration that EMACS_INT can hold
non-fixnum values due to bits needed for the Lisp_Object tag. You can
disregard my previous diff. The following might work, though I'm not
sure how applicable this is now to the jansson library:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sizeof --]
[-- Type: text/x-patch, Size: 638 bytes --]

diff --git a/src/lisp.h b/src/lisp.h
index c2cb89de9d..c45e35178e 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -2661,8 +2661,10 @@ make_uint (uintmax_t n)
 }
 
 /* Return a Lisp integer equal to the value of the C integer EXPR.  */
-#define INT_TO_INTEGER(expr) \
-  (EXPR_SIGNED (expr) ? make_int (expr) : make_uint (expr))
+#define INT_TO_INTEGER(expr)                                    \
+  (sizeof (expr) < sizeof (EMACS_INT)                           \
+   ? make_fixnum (expr)                                         \
+   : (EXPR_SIGNED (expr) ? make_int (expr) : make_uint (expr)))
 
 \f
 /* Forwarding pointer to an int variable.

[-- Attachment #3: Type: text/plain, Size: 246 bytes --]


The above assumes that INTTYPEBITS is less than 8, which I suppose could
be put in there as well as a sanity check.

> Not sure what you meant by "usual case".

I just meant the case where json_int_t does not have a greater size than
EMACS_INT.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 22.04.2019 19:22, Eli Zaretskii wrote:
> You and others are welcome to profile the code looking
> for opportunities to optimize even more, but I'd be surprised if you
> find something significant.  In particular, UTF-8 decoding validates
> as part of the decoding loop, and is already optimized for ASCII, so
> it will be interesting to see what kind of gains we could have in
> json_make_string by testing for ASCII up front.

I wonder what conclusions you're going to make from the current results.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 22.04.2019 22:52, Alex Gramiak wrote:
> The following might work, though I'm not
> sure how applicable this is now to the jansson library:

Peformance patches are 200% better when they come together with 
benchmark results.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: sebastien@chapu.is, yyoncho@gmail.com, 31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 22 Apr 2019 22:55:40 +0300
> 
> I wonder what conclusions you're going to make from the current results.

Ask me when I see them.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 22.04.2019 20:12, Eli Zaretskii wrote:

> Like this:
> 
> diff --git a/src/json.c b/src/json.c
> index 16500bc..48aeccf 100644
> --- a/src/json.c
> +++ b/src/json.c
> @@ -225,8 +225,12 @@ json_has_suffix (const char *string, const char *suffix)
>   static Lisp_Object
>   json_make_string (const char *data, ptrdiff_t size)
>   {
> -  return code_convert_string (make_specified_string (data, -1, size, false),
> -                              Qutf_8_unix, Qt, false, true, true);
> +  struct coding_system coding;
> +  setup_coding_system (Qutf_8_unix, &coding);
> +  coding.mode |= CODING_MODE_LAST_BLOCK;
> +  coding.source = data;
> +  decode_coding_object (&coding, Qnil, 0, 0, size, size, Qt);
> +  return coding.dst_object;
>   }
>   
>   /* Create a multibyte Lisp string from the NUL-terminated UTF-8
> 
> 
> How does this fare in your benchmarks?

It's not nothing, but it doesn't remotely compare to avoiding the 
decoder entirely. Here's a table with different patches and 
corresponding timings. Your last patch is at the end.

I have to admit that somehow my previous measurement of the "none" 
performance was off, maybe due to cold disk cache, maybe due to busy CPU 
or whatever. So it's not 51s but faster.

| patch                        | runtime (s) |
|------------------------------+-------------|
| none (current master)        |        36.8 |
| no validation                |        21.7 |
| ascii_only check             |        21.8 |
| using coding_system directly |        34.5 |

I have also tried your suggested scenario (where the file is read only 
once), but it made the "none" case faster by 2 seconds. So I decided 
against repeating all tests with it.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 22.04.2019 20:24, Eli Zaretskii wrote:

> Yes, but I'm slightly surprised why you loop from the end of the
> string and not from the beginning.

To avoid creating an additional pointer variable.

> I guess that's expected when the strings in JSON are short enough.

Longer strings take a proportional amount of time to encode, though 
(only 2x as fast per character, IIRC).

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 22.04.2019 20:26, Eli Zaretskii wrote:
> I don't think we should abort, we don't do that anywhere else.

The key word here is "validation". If certain folks in this discussion 
are right, libjansson always returns valid utf-8 encoded strings, or 
intends to.

So if we implement fast validation for multibyte strings as well, the 
"else" branch should never be taken, and it would make sense to abort in 
that case. Not necessarily abort as in "exit Emacs", but maybe signal an 
error. The libjansson developers might in the end be thankful for bug 
reports.

bug#31138: Native json slower than json.el

[Alex Gramiak]

[-- Attachment #1: Type: text/plain, Size: 1263 bytes --]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 22.04.2019 22:52, Alex Gramiak wrote:
>> The following might work, though I'm not
>> sure how applicable this is now to the jansson library:
>
> Peformance patches are 200% better when they come together with benchmark
> results.

Right, I should have explained: I meant that since AFAIU the size of
json_int_t is usually going to be the same size as EMACS_INT, the same
code will usually be generated for json.c. It might bring a very slight
benefit to other parts of Emacs, but not worth fussing over.

I tried making a couple patches in other areas to json.c that I hoped
would slightly improve performance, which I'll include here for
posterity. The first one uses json_dumpb over json_dumps, but from what
I can tell using it is either slower or just within error. The second
uses the *_nocheck functions where applicable, but the results seemed to
be quite similar. I suppose this means that jansson's utf-8 checking is
performant.


Benchmark for the first patch:
  (setq v ["\nasdфыв\u001f\u007ffgh\t"])
  
  (progn (garbage-collect)
         (benchmark-run-compiled 100000
           (json-serialize v)))

Results for the first patch: 1.1050s applied vs. 1.1078s without.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: json_dumpb --]
[-- Type: text/x-patch, Size: 3001 bytes --]

diff --git a/configure.ac b/configure.ac
index b4a9b30691..2dba99838f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2982,6 +2982,10 @@ AC_DEFUN
   if test "${HAVE_JSON}" = yes; then
     AC_DEFINE(HAVE_JSON, 1, [Define if using Jansson.])
     JSON_OBJ=json.o
+    OLDLIBS="$LIBS"
+    LIBS="$LIBS $JSON_LIBS"
+    AC_CHECK_FUNCS(json_dumpb)
+    LIBS="$OLDLIBS"
   fi
 
   # Windows loads libjansson dynamically
diff --git a/src/json.c b/src/json.c
index 928825e034..361581c2f4 100644
--- a/src/json.c
+++ b/src/json.c
@@ -31,7 +31,7 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 #include "coding.h"
 
 #define JSON_HAS_ERROR_CODE (JANSSON_VERSION_HEX >= 0x020B00)
-
+#undef HAVE_JSON_DUMPB
 #ifdef WINDOWSNT
 # include <windows.h>
 # include "w32common.h"
@@ -53,6 +53,11 @@ DEF_DLL_FN (json_t *, json_integer, (json_int_t value));
 DEF_DLL_FN (json_t *, json_real, (double value));
 DEF_DLL_FN (json_t *, json_stringn, (const char *value, size_t len));
 DEF_DLL_FN (char *, json_dumps, (const json_t *json, size_t flags));
+# ifdef HAVE_JSON_DUMPB
+DEF_DLL_FN (void, json_dumpb, (const json_t *json, char *buffer,
+                               size_t size,
+                               size_t flags));
+# endif
 DEF_DLL_FN (int, json_dump_callback,
 	    (const json_t *json, json_dump_callback_t callback, void *data,
 	     size_t flags));
@@ -104,6 +109,9 @@ init_json_functions (void)
   LOAD_DLL_FN (library, json_real);
   LOAD_DLL_FN (library, json_stringn);
   LOAD_DLL_FN (library, json_dumps);
+# ifdef HAVE_JSON_DUMPB
+  LOAD_DLL_FN (library, json_dumpb);
+# endif
   LOAD_DLL_FN (library, json_dump_callback);
   LOAD_DLL_FN (library, json_integer_value);
   LOAD_DLL_FN (library, json_real_value);
@@ -605,6 +613,10 @@ usage: (json-serialize OBJECT &rest ARGS)  */)
 {
   ptrdiff_t count = SPECPDL_INDEX ();
 
+#ifdef HAVE_JSON_DUMPB
+  USE_SAFE_ALLOCA;
+#endif
+
 #ifdef WINDOWSNT
   if (!json_initialized)
     {
@@ -625,16 +637,34 @@ usage: (json-serialize OBJECT &rest ARGS)  */)
   json_parse_args (nargs - 1, args + 1, &conf, false);
 
   json_t *json = lisp_to_json_toplevel (args[0], &conf);
-  record_unwind_protect_ptr (json_release_object, json);
 
+#ifdef HAVE_JSON_DUMPB
+  /* Called with NULL to determine the size of buffer needed.  */
+  size_t size = json_dumpb (json, NULL, 0, 0);
+  char *string = SAFE_ALLOCA (size);
+
+  size = json_dumpb (json, string, size, JSON_COMPACT);
+
+  json_release_object (json);
+
+  if (!size)
+    json_out_of_memory ();
+
+
+  return SAFE_FREE_UNBIND_TO (count, json_make_string (string, size));
+#else
   /* If desired, we might want to add the following flags:
      JSON_DECODE_ANY, JSON_ALLOW_NUL.  */
   char *string = json_dumps (json, JSON_COMPACT);
+
+  json_release_object (json);
+
   if (string == NULL)
     json_out_of_memory ();
   record_unwind_protect_ptr (json_free, string);
 
   return unbind_to (count, json_build_string (string));
+#endif
 }
 
 struct json_buffer_and_size

[-- Attachment #3: Type: text/plain, Size: 467 bytes --]


Benchmark for the second patch:

  (setq v (with-current-buffer (find-file-noselect
                                "~/emacs/test/src/json-tests.el")
            (vector (buffer-substring-no-properties (point-min)
                                                    (point-max)))))

    (progn (garbage-collect)
           (benchmark-run-compiled 1000
             (json-serialize v)))
           
Results for the second patch: 0.7815s applied vs. 0.7962s without.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: nocheck --]
[-- Type: text/x-patch, Size: 3636 bytes --]

diff --git a/src/json.c b/src/json.c
index 928825e034..3aa6be7212 100644
--- a/src/json.c
+++ b/src/json.c
@@ -46,12 +46,14 @@ DEF_DLL_FN (size_t, json_array_size, (const json_t *array));
 DEF_DLL_FN (json_t *, json_object, (void));
 DEF_DLL_FN (int, json_object_set_new,
 	    (json_t *object, const char *key, json_t *value));
+DEF_DLL_FN (int, json_object_set_new_nocheck,
+	    (json_t *object, const char *key, json_t *value));
 DEF_DLL_FN (json_t *, json_null, (void));
 DEF_DLL_FN (json_t *, json_true, (void));
 DEF_DLL_FN (json_t *, json_false, (void));
 DEF_DLL_FN (json_t *, json_integer, (json_int_t value));
 DEF_DLL_FN (json_t *, json_real, (double value));
-DEF_DLL_FN (json_t *, json_stringn, (const char *value, size_t len));
+DEF_DLL_FN (json_t *, json_stringn_nocheck, (const char *value, size_t len));
 DEF_DLL_FN (char *, json_dumps, (const json_t *json, size_t flags));
 DEF_DLL_FN (int, json_dump_callback,
 	    (const json_t *json, json_dump_callback_t callback, void *data,
@@ -97,12 +99,13 @@ init_json_functions (void)
   LOAD_DLL_FN (library, json_array_size);
   LOAD_DLL_FN (library, json_object);
   LOAD_DLL_FN (library, json_object_set_new);
+  LOAD_DLL_FN (library, json_object_set_new_nocheck);
   LOAD_DLL_FN (library, json_null);
   LOAD_DLL_FN (library, json_true);
   LOAD_DLL_FN (library, json_false);
   LOAD_DLL_FN (library, json_integer);
   LOAD_DLL_FN (library, json_real);
-  LOAD_DLL_FN (library, json_stringn);
+  LOAD_DLL_FN (library, json_stringn_nocheck);
   LOAD_DLL_FN (library, json_dumps);
   LOAD_DLL_FN (library, json_dump_callback);
   LOAD_DLL_FN (library, json_integer_value);
@@ -131,12 +134,13 @@ init_json_functions (void)
 #define json_array_size fn_json_array_size
 #define json_object fn_json_object
 #define json_object_set_new fn_json_object_set_new
+#define json_object_set_new_nocheck fn_json_object_set_new_nocheck
 #define json_null fn_json_null
 #define json_true fn_json_true
 #define json_false fn_json_false
 #define json_integer fn_json_integer
 #define json_real fn_json_real
-#define json_stringn fn_json_stringn
+#define json_stringn_nocheck fn_json_stringn_nocheck
 #define json_dumps fn_json_dumps
 #define json_dump_callback fn_json_dump_callback
 #define json_integer_value fn_json_integer_value
@@ -394,9 +398,10 @@ lisp_to_json_toplevel_1 (Lisp_Object lisp,
                table test is not `equal'.  */
             if (json_object_get (json, key_str) != NULL)
               wrong_type_argument (Qjson_value_p, lisp);
-            int status = json_object_set_new (json, key_str,
-                                              lisp_to_json (HASH_VALUE (h, i),
-                                                            conf));
+            int status
+              = json_object_set_new_nocheck (json, key_str,
+                                             lisp_to_json (HASH_VALUE (h, i),
+                                                           conf));
             if (status == -1)
               {
                 /* A failure can be caused either by an invalid key or
@@ -508,12 +513,9 @@ lisp_to_json (Lisp_Object lisp, struct json_configuration *conf)
   else if (STRINGP (lisp))
     {
       Lisp_Object encoded = json_encode (lisp);
-      json_t *json = json_stringn (SSDATA (encoded), SBYTES (encoded));
+      json_t *json = json_stringn_nocheck (SSDATA (encoded), SBYTES (encoded));
       if (json == NULL)
         {
-          /* A failure can be caused either by an invalid string or by
-             low memory.  */
-          json_check_utf8 (encoded);
           json_out_of_memory ();
         }
       return json;

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: p.stephani2@gmail.com, sebastien@chapu.is, yyoncho@gmail.com,
>  31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 23 Apr 2019 01:23:44 +0300
> 
> On 22.04.2019 20:26, Eli Zaretskii wrote:
> > I don't think we should abort, we don't do that anywhere else.
> 
> The key word here is "validation". If certain folks in this discussion 
> are right, libjansson always returns valid utf-8 encoded strings, or 
> intends to.
> 
> So if we implement fast validation for multibyte strings as well, the 
> "else" branch should never be taken, and it would make sense to abort in 
> that case. Not necessarily abort as in "exit Emacs", but maybe signal an 
> error. The libjansson developers might in the end be thankful for bug 
> reports.

The interests of libjansson developers aside, I disagree that
low-level text decoding functionality should signal an error, let
alone abort.  It is up to the application to decide whether having raw
bytes in strings is legit or not.  This is our policy everywhere,
including when UTF-8 is the standard encoding, so I don't think this
case is special in any way.

bug#31138: Native json slower than json.el

[Philipp Stephani]

Am Di., 23. Apr. 2019 um 08:00 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
>
> > Cc: p.stephani2@gmail.com, sebastien@chapu.is, yyoncho@gmail.com,
> >  31138@debbugs.gnu.org
> > From: Dmitry Gutov <dgutov@yandex.ru>
> > Date: Tue, 23 Apr 2019 01:23:44 +0300
> >
> > On 22.04.2019 20:26, Eli Zaretskii wrote:
> > > I don't think we should abort, we don't do that anywhere else.
> >
> > The key word here is "validation". If certain folks in this discussion
> > are right, libjansson always returns valid utf-8 encoded strings, or
> > intends to.
> >
> > So if we implement fast validation for multibyte strings as well, the
> > "else" branch should never be taken, and it would make sense to abort in
> > that case. Not necessarily abort as in "exit Emacs", but maybe signal an
> > error. The libjansson developers might in the end be thankful for bug
> > reports.
>
> The interests of libjansson developers aside, I disagree that
> low-level text decoding functionality should signal an error, let
> alone abort.  It is up to the application to decide whether having raw
> bytes in strings is legit or not.  This is our policy everywhere,
> including when UTF-8 is the standard encoding, so I don't think this
> case is special in any way.

It's special in the sense that both the JSON RFC and Jansson require
UTF-8 for interchange, so we don't have to deal with raw bytes and can
assume all text is UTF-8-encoded. The test suite contains a few tests
to ensure that passing non-UTF-8-strings always results in an error.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: p.stephani2@gmail.com, sebastien@chapu.is, yyoncho@gmail.com,
>  31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 23 Apr 2019 00:03:30 +0300
> 
> On 22.04.2019 20:24, Eli Zaretskii wrote:
> 
> > Yes, but I'm slightly surprised why you loop from the end of the
> > string and not from the beginning.
> 
> To avoid creating an additional pointer variable.

I don't think it matters, and looping forward is more natural and may
even be slightly faster.

> > I guess that's expected when the strings in JSON are short enough.
> 
> Longer strings take a proportional amount of time to encode, though 
> (only 2x as fast per character, IIRC).

I was talking about decoding.  Assuming that decode_coding_utf_8 has
some setup overhead before it starts the loop of processing the bytes,
that overhead will become less significant with longer strings.  And
indeed, if I make the strings in large.json be 10K characters (can
this happen in real-life JSONs?), the speedup from using
make_specified_string for valid UTF-8 input goes down to just 40% for
unoptimized builds and 20% for optimized (see the timing data below).
But it's still faster even for such large strings, so I installed a
variant of what we were discussing.

Comparing with json.el shows that we've got 8-fold to ten-fold speedup
in optimized builds.

Here are my timings for the various variants ("large" means with JSON
input where all strings were enlarged to 10K characters):

  variant                       | unoptimized | optimized
  ------------------------------+-------------+----------
  curent master                 |    3.563    |   0.664
  curent master, large          |  174.0      |  43.34
  no validation                 |    0.980    |   0.326
  no validation, large          |  105.1      |  33.13
  coding_system directly        |    2.962    |   0.660
  coding_system directly, large |  173.4      |  43.19
  UTF-8 validation              |    0.980    |   0.334
  UTF-8 validation, large       |  105.9      |  34.36

In all cases, the times are from 10 benchmark loops, after subtracting
the time used by GC.

Thanks.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Tue, 23 Apr 2019 11:46:58 +0200
> Cc: Dmitry Gutov <dgutov@yandex.ru>, Sébastien Chapuis <sebastien@chapu.is>, 
> 	yyoncho <yyoncho@gmail.com>, 31138@debbugs.gnu.org
> 
> > The interests of libjansson developers aside, I disagree that
> > low-level text decoding functionality should signal an error, let
> > alone abort.  It is up to the application to decide whether having raw
> > bytes in strings is legit or not.  This is our policy everywhere,
> > including when UTF-8 is the standard encoding, so I don't think this
> > case is special in any way.
> 
> It's special in the sense that both the JSON RFC and Jansson require
> UTF-8 for interchange, so we don't have to deal with raw bytes and can
> assume all text is UTF-8-encoded.

We agreed to disagree about this long ago.  Can we please stop arguing
about this time and again?

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 23.04.2019 13:38, Eli Zaretskii wrote:
>> It's special in the sense that both the JSON RFC and Jansson require
>> UTF-8 for interchange, so we don't have to deal with raw bytes and can
>> assume all text is UTF-8-encoded.
> We agreed to disagree about this long ago.  Can we please stop arguing
> about this time and again?

Even if we don't intend to trust Jansson on this, returning valid UTF-8 
is in the contract. So it should be perfectly reasonable to discard any 
invalid UTF-8 strings it returns and signal an error, rather than try to 
somehow decode them.

Leading to easier to follow code, for one thing. Right now it's not 
obvious that the "else" branch should never be taken in practice.

At this point I'm out of arguments. It is, of course, your call.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 23.04.2019 13:22, Eli Zaretskii wrote:

>>> Yes, but I'm slightly surprised why you loop from the end of the
>>> string and not from the beginning.
>>
>> To avoid creating an additional pointer variable.
> 
> I don't think it matters, and looping forward is more natural and may
> even be slightly faster.

OK. It was mostly a matter of taste for me anyway. (I would be 
interested in any examples of "slightly faster", though).

>>> I guess that's expected when the strings in JSON are short enough.
>>
>> Longer strings take a proportional amount of time to encode, though
>> (only 2x as fast per character, IIRC).
> 
> I was talking about decoding.  Assuming that decode_coding_utf_8 has
> some setup overhead before it starts the loop of processing the bytes,
> that overhead will become less significant with longer strings.  And
> indeed, if I make the strings in large.json be 10K characters (can
> this happen in real-life JSONs?),

Everything can happen, but I'm not aware of a particular application.

> the speedup from using
> make_specified_string for valid UTF-8 input goes down to just 40% for
> unoptimized builds and 20% for optimized (see the timing data below).
> But it's still faster even for such large strings, so I installed a
> variant of what we were discussing.

Thank you.

And for small strings, your numbers seem even more encouraging than mine.

> Comparing with json.el shows that we've got 8-fold to ten-fold speedup
> in optimized builds.
> 
> Here are my timings for the various variants ("large" means with JSON
> input where all strings were enlarged to 10K characters):
> 
>    variant                       | unoptimized | optimized
>    ------------------------------+-------------+----------
>    curent master                 |    3.563    |   0.664
>    curent master, large          |  174.0      |  43.34
>    no validation                 |    0.980    |   0.326
>    no validation, large          |  105.1      |  33.13
>    coding_system directly        |    2.962    |   0.660
>    coding_system directly, large |  173.4      |  43.19
>    UTF-8 validation              |    0.980    |   0.334
>    UTF-8 validation, large       |  105.9      |  34.36

0.334 vs 0.644, I like that. :-)

> In all cases, the times are from 10 benchmark loops, after subtracting
> the time used by GC.

I figured we might be saving a bit on GC pauses as well (doing and 
allocating less stuff), but they are harder to time, of course.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 23.04.2019 6:06, Alex Gramiak wrote:

> I tried making a couple patches in other areas to json.c that I hoped
> would slightly improve performance, which I'll include here for
> posterity. The first one uses json_dumpb over json_dumps, but from what
> I can tell using it is either slower or just within error. The second
> uses the *_nocheck functions where applicable, but the results seemed to
> be quite similar. I suppose this means that jansson's utf-8 checking is
> performant.

You might also want to try it on larger JSON documents, documents that 
include more numbers, etc.

With a JSON string this small, the benchmark loop might be taking most 
of the runtime.

Anyway, thank you for the try and the benchmarking. At this point I'm 
still hoping for larger gains, and they'll probably come from some 
higher-level changes.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 22.04.2019 23:28, Eli Zaretskii wrote:
>> Cc: sebastien@chapu.is, yyoncho@gmail.com, 31138@debbugs.gnu.org
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Date: Mon, 22 Apr 2019 22:55:40 +0300
>>
>> I wonder what conclusions you're going to make from the current results.
> 
> Ask me when I see them.

Asking. :-)

Though you have probably mentioned them in the other email (significant 
up-front cost for initializing the decoder, right?). I wonder if having 
this optimization built-in in decode-coding-string or thereabouts could 
give us performance gains somewhere else as well.

And this particular bug report can probably be closed.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: sebastien@chapu.is, yyoncho@gmail.com, 31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 23 Apr 2019 14:52:21 +0300
> 
> I wonder if having this optimization built-in in
> decode-coding-string or thereabouts could give us performance gains
> somewhere else as well.

I thought about this.  It could make sense to have a UTF-8 specific
function to encode and decode strings.  With encodings other than
UTF-8 it becomes trickier, and probably likewise with buffer text,
where we need to take the gap into account.

What applications do we have where en/decoding strings has critical
effect on performance?

> And this particular bug report can probably be closed.

Assuming the others confirm.

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 136 bytes --]

Hi Eli,

What applications do we have where en/decoding strings has critical
> effect on performance?
>

Dynamic modules.

Thanks,
Ivan

[-- Attachment #2: Type: text/html, Size: 410 bytes --]

bug#31138: Native json slower than json.el

[Sébastien Chapuis]

> And this particular bug report can probably be closed.

I agree, thank you all for working on this.

> What applications do we have where en/decoding strings has critical
effect on performance?

I am thinking to module_make_string

Le mar. 23 avr. 2019 à 20:15, Eli Zaretskii <eliz@gnu.org> a écrit :
>
> > Cc: sebastien@chapu.is, yyoncho@gmail.com, 31138@debbugs.gnu.org
> > From: Dmitry Gutov <dgutov@yandex.ru>
> > Date: Tue, 23 Apr 2019 14:52:21 +0300
> >
> > I wonder if having this optimization built-in in
> > decode-coding-string or thereabouts could give us performance gains
> > somewhere else as well.
>
> I thought about this.  It could make sense to have a UTF-8 specific
> function to encode and decode strings.  With encodings other than
> UTF-8 it becomes trickier, and probably likewise with buffer text,
> where we need to take the gap into account.
>
> What applications do we have where en/decoding strings has critical
> effect on performance?
>
> > And this particular bug report can probably be closed.
>
> Assuming the others confirm.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Tue, 23 Apr 2019 15:37:33 +0300
> Cc: Dmitry Gutov <dgutov@yandex.ru>, Sébastien Chapuis <sebastien@chapu.is>, 
> 	31138@debbugs.gnu.org
> 
>  What applications do we have where en/decoding strings has critical
>  effect on performance?
> 
> Dynamic modules.

Modules doing what?

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: Sébastien Chapuis <sebastien@chapu.is>
> Date: Tue, 23 Apr 2019 20:37:54 +0800
> Cc: Dmitry Gutov <dgutov@yandex.ru>, Ivan Yonchovski <yyoncho@gmail.com>, 31138@debbugs.gnu.org
> 
> I am thinking to module_make_string

When/where is that performance-critical?

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: p.stephani2@gmail.com, sebastien@chapu.is, yyoncho@gmail.com,
>  31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 23 Apr 2019 14:39:50 +0300
> 
> On 23.04.2019 13:22, Eli Zaretskii wrote:
> 
> >>> Yes, but I'm slightly surprised why you loop from the end of the
> >>> string and not from the beginning.
> >>
> >> To avoid creating an additional pointer variable.
> > 
> > I don't think it matters, and looping forward is more natural and may
> > even be slightly faster.
> 
> OK. It was mostly a matter of taste for me anyway. (I would be 
> interested in any examples of "slightly faster", though).

It's a hunch.  I think modern processors and their caches deal with
incrementing better than with decrementing.  Maybe I'm wrong.

> significant up-front cost for initializing the decoder, right?

That, and the general disadvantage of going over the text twice
instead of just once.

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 167 bytes --]

Hi Eli,

Modules doing what?
>

Modules that have to create a lot of strings(e. g. parser module aiming to
replace elisp code to get a performance improvement).

Ivan

[-- Attachment #2: Type: text/html, Size: 421 bytes --]

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 23.04.2019 15:15, Eli Zaretskii wrote:
> I thought about this.  It could make sense to have a UTF-8 specific
> function to encode and decode strings.  With encodings other than
> UTF-8 it becomes trickier, and probably likewise with buffer text,
> where we need to take the gap into account.

Doing that for buffer text as well might be helpful. Other encodings are 
much less of a priority, I would say.

> What applications do we have where en/decoding strings has critical
> effect on performance?

It wouldn't be critical most of the time, but even a few % performance 
improvement across the board, basically for free, might be welcome.

So that's why I mentioned decode-coding-string (though 
code_convert_string would be a better choice; or decode_coding_object?), 
as opposed to creating a new specialized function.

What I can understand from our testing, this kind of change improves 
performance for all kinds of strings when the source encoding is 
utf_8_unix. Even for large ones (despite you expecting otherwise). The 
only kinds of input where this should result in a (likely minor) 
slowdown would be ones where the contents do not correspond to the 
declared encoding.

Again, the patch, or several, shouldn't be particularly hard to write, 
and we can try them out with different scenarios.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Tue, 23 Apr 2019 16:27:59 +0300
> Cc: Dmitry Gutov <dgutov@yandex.ru>, Sébastien Chapuis <sebastien@chapu.is>, 
> 	31138@debbugs.gnu.org
> 
>  Modules doing what?
> 
> Modules that have to create a lot of strings(e. g. parser module aiming to replace elisp code to get a
> performance improvement).

If they use JSON, the solution is already here.

I don't think we should discuss hypothetical examples, because we can
never time and profile those to see whether string import into Emacs
is indeed the bottleneck.

bug#31138: Native json slower than json.el

[Philipp Stephani]

Am Di., 23. Apr. 2019 um 16:25 Uhr schrieb Dmitry Gutov <dgutov@yandex.ru>:
>
> On 23.04.2019 15:15, Eli Zaretskii wrote:
> > I thought about this.  It could make sense to have a UTF-8 specific
> > function to encode and decode strings.  With encodings other than
> > UTF-8 it becomes trickier, and probably likewise with buffer text,
> > where we need to take the gap into account.
>
> Doing that for buffer text as well might be helpful. Other encodings are
> much less of a priority, I would say.
>
> > What applications do we have where en/decoding strings has critical
> > effect on performance?
>
> It wouldn't be critical most of the time, but even a few % performance
> improvement across the board, basically for free, might be welcome.
>
> So that's why I mentioned decode-coding-string (though
> code_convert_string would be a better choice; or decode_coding_object?),
> as opposed to creating a new specialized function.
>
> What I can understand from our testing, this kind of change improves
> performance for all kinds of strings when the source encoding is
> utf_8_unix. Even for large ones (despite you expecting otherwise). The
> only kinds of input where this should result in a (likely minor)
> slowdown would be ones where the contents do not correspond to the
> declared encoding.
>
> Again, the patch, or several, shouldn't be particularly hard to write,
> and we can try them out with different scenarios.


For starters, the module code in emacs-module.c (e.g.
module_make_string) has essentially the same requirements. So we could
at least move json_make_string, json_build_string, and json_encode
into coding.[ch] (and rename them).

bug#31138: Native json slower than json.el

[Andy Moreton]

On Tue 23 Apr 2019, Eli Zaretskii wrote:

>> Cc: sebastien@chapu.is, yyoncho@gmail.com, 31138@debbugs.gnu.org
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Date: Tue, 23 Apr 2019 14:52:21 +0300
>> 
>> I wonder if having this optimization built-in in
>> decode-coding-string or thereabouts could give us performance gains
>> somewhere else as well.
>
> I thought about this.  It could make sense to have a UTF-8 specific
> function to encode and decode strings.  With encodings other than
> UTF-8 it becomes trickier, and probably likewise with buffer text,
> where we need to take the gap into account.
>
> What applications do we have where en/decoding strings has critical
> effect on performance?

One possible candidate is the LSP packages from ELPA/MELPA (eglot,
lsp-mode, etc) which use JSON-RPC to communicate with the external
language servers. Their interactive performance may be sensitive to JSON
overheads.

I don't yet use these tools, so perhaps someone who does can indicate if
faster JSON handling is important for these tools.

    AndyM

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: sebastien@chapu.is, yyoncho@gmail.com, 31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 23 Apr 2019 17:22:34 +0300
> 
> On 23.04.2019 15:15, Eli Zaretskii wrote:
> > I thought about this.  It could make sense to have a UTF-8 specific
> > function to encode and decode strings.  With encodings other than
> > UTF-8 it becomes trickier, and probably likewise with buffer text,
> > where we need to take the gap into account.
> 
> Doing that for buffer text as well might be helpful.

In what use cases would this be helpful?  Most cases of decoding text
in a buffer happen when we read text from files, where we already have
an internal optimization for plain ASCII files.  We could perhaps try
a similar optimization for UTF-8 instead of just ASCII.

Use cases where we read without decoding and then decode buffer
contents "by hand" are relatively rare, certainly when the stuff to
decode is so large that the performance gains will be tangible.

> So that's why I mentioned decode-coding-string (though 
> code_convert_string would be a better choice; or decode_coding_object?), 
> as opposed to creating a new specialized function.

code_convert_string also handles encoding, though.

> What I can understand from our testing, this kind of change improves 
> performance for all kinds of strings when the source encoding is 
> utf_8_unix. Even for large ones (despite you expecting otherwise).

I tested 10K strings, and the advantage there already becomes
relatively small.  10K characters may be a lot for strings, but it
isn't for buffers.  The optimization we use as part of decoding
insert-file-contents avoids the problem by inserting the ASCII part
directly and starting to decode from the first non-ASCII character.
With strings and with text already in the buffer this is not currently
possible, or at least not easily.

> Again, the patch, or several, shouldn't be particularly hard to write, 
> and we can try them out with different scenarios.

If someone wants to work on such patches, I'm sure they will be
welcome.  But we should have clear use cases and good test cases to
time them, IMO.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Tue, 23 Apr 2019 15:50:57 +0100
> 
> > What applications do we have where en/decoding strings has critical
> > effect on performance?
> 
> One possible candidate is the LSP packages from ELPA/MELPA (eglot,
> lsp-mode, etc) which use JSON-RPC to communicate with the external
> language servers. Their interactive performance may be sensitive to JSON
> overheads.
> 
> I don't yet use these tools, so perhaps someone who does can indicate if
> faster JSON handling is important for these tools.

JSON performance already got sped up: that's what this bug report and
the associated commits were all about.  I was asking which other
applications that do NOT use JSON might have string conversion a
bottleneck.

Or did I misunderstand what you were saying?

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Tue, 23 Apr 2019 16:40:10 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>, Sébastien Chapuis <sebastien@chapu.is>, 
> 	yyoncho <yyoncho@gmail.com>, 31138@debbugs.gnu.org
> 
> For starters, the module code in emacs-module.c (e.g.
> module_make_string) has essentially the same requirements. So we could
> at least move json_make_string, json_build_string, and json_encode
> into coding.[ch] (and rename them).

If we want to make this more general-purpose, we should time the code
with very long strings.  It could be that beyond some length the UTF-8
verification, if it fails, will make the conversion slower, in which
case we probably should not take the shortcut beyond that length
limit.

Or we might document that non-UTF-8 strings will be slower to convert,
if that's acceptable.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Date: Tue, 23 Apr 2019 18:09:55 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: sebastien@chapu.is, dgutov@yandex.ru, yyoncho@gmail.com,
> 	31138@debbugs.gnu.org
> 
> If we want to make this more general-purpose, we should time the code
> with very long strings.  It could be that beyond some length the UTF-8
> verification, if it fails, will make the conversion slower, in which
> case we probably should not take the shortcut beyond that length
> limit.

Another minor issue to consider is that moving json_make_string into
another file might make JSON processing somewhat slower, because the
function will/might no longer be inlined.  Not sure if this is
important enough, though.  Perhaps timing the results will tell.

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 1061 bytes --]

Hi Eli,

Another posibility is to have two string convertion methods.

Thanks,
Ivan

On Tue, Apr 23, 2019 at 6:10 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Tue, 23 Apr 2019 16:40:10 +0200
> > Cc: Eli Zaretskii <eliz@gnu.org>, Sébastien Chapuis <sebastien@chapu.is>,
>
> >       yyoncho <yyoncho@gmail.com>, 31138@debbugs.gnu.org
> >
> > For starters, the module code in emacs-module.c (e.g.
> > module_make_string) has essentially the same requirements. So we could
> > at least move json_make_string, json_build_string, and json_encode
> > into coding.[ch] (and rename them).
>
> If we want to make this more general-purpose, we should time the code
> with very long strings.  It could be that beyond some length the UTF-8
> verification, if it fails, will make the conversion slower, in which
> case we probably should not take the shortcut beyond that length
> limit.
>
> Or we might document that non-UTF-8 strings will be slower to convert,
> if that's acceptable.
>

[-- Attachment #2: Type: text/html, Size: 1755 bytes --]

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Tue, 23 Apr 2019 18:36:18 +0300
> Cc: Philipp Stephani <p.stephani2@gmail.com>, Dmitry Gutov <dgutov@yandex.ru>, 
> 	Sébastien Chapuis <sebastien@chapu.is>, 
> 	31138@debbugs.gnu.org
> 
> Another posibility is to have two string convertion methods.

We already have them, that's what json_make_string uses internally.
Right?

bug#31138: Native json slower than json.el

[yyoncho]

[-- Attachment #1: Type: text/plain, Size: 571 bytes --]

I am talking about module_make_utf8_string and module_make_string.

Thanks,
Ivan

On Tue, Apr 23, 2019 at 6:39 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: yyoncho <yyoncho@gmail.com>
> > Date: Tue, 23 Apr 2019 18:36:18 +0300
> > Cc: Philipp Stephani <p.stephani2@gmail.com>, Dmitry Gutov <
> dgutov@yandex.ru>,
> >       Sébastien Chapuis <sebastien@chapu.is>,
> >       31138@debbugs.gnu.org
> >
> > Another posibility is to have two string convertion methods.
>
> We already have them, that's what json_make_string uses internally.
> Right?
>

[-- Attachment #2: Type: text/html, Size: 1243 bytes --]

bug#31138: Native json slower than json.el

[Andy Moreton]

On Tue 23 Apr 2019, Eli Zaretskii wrote:

>> From: Andy Moreton <andrewjmoreton@gmail.com>
>> Date: Tue, 23 Apr 2019 15:50:57 +0100
>> 
>> > What applications do we have where en/decoding strings has critical
>> > effect on performance?
>> 
>> One possible candidate is the LSP packages from ELPA/MELPA (eglot,
>> lsp-mode, etc) which use JSON-RPC to communicate with the external
>> language servers. Their interactive performance may be sensitive to JSON
>> overheads.
>> 
>> I don't yet use these tools, so perhaps someone who does can indicate if
>> faster JSON handling is important for these tools.
>
> JSON performance already got sped up: that's what this bug report and
> the associated commits were all about.  I was asking which other
> applications that do NOT use JSON might have string conversion a
> bottleneck.

Ah - my bad.

> Or did I misunderstand what you were saying?

No, I did not read enough context.

    AndyM

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 23.04.2019 18:09, Eli Zaretskii wrote:

> If we want to make this more general-purpose, we should time the code
> with very long strings.  It could be that beyond some length the UTF-8
> verification, if it fails, will make the conversion slower, in which
> case we probably should not take the shortcut beyond that length
> limit.

If it fails, yes. Although it might still be an insignificant fraction 
of the total time it takes (verification is O(N), and conversion is O(N) 
as well).

> Or we might document that non-UTF-8 strings will be slower to convert,
> if that's acceptable.

*Invalid* UTF-8 strings. Strings that aren't actually UTF-8, even though 
the caller declares them to be.

I think this can be a good (and simple) tradeoff.

bug#31138: Native json slower than json.el

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > The interests of libjansson developers aside, I disagree that
  > low-level text decoding functionality should signal an error, let
  > alone abort.  It is up to the application to decide whether having raw
  > bytes in strings is legit or not.

I agree.

-- 
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: sebastien@chapu.is, yyoncho@gmail.com, 31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 24 Apr 2019 01:34:37 +0300
> 
> > Or we might document that non-UTF-8 strings will be slower to convert,
> > if that's acceptable.
> 
> *Invalid* UTF-8 strings. Strings that aren't actually UTF-8, even though 
> the caller declares them to be.

We are talking about a function that expects UTF-8 strings, so
"non-UTF-8" and "invalid UTF-8" are 2 different ways of saying the
same thing.

However, note that various 7-bit encodings will be taken by such an
interface as valid UTF-8, since all the bytes are 7-bit ASCII.
There's nothing we can do about that, just something to keep in mind
(it isn't supposed to happen in correct usage).

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 24.04.2019 9:20, Eli Zaretskii wrote:

> We are talking about a function that expects UTF-8 strings, so
> "non-UTF-8" and "invalid UTF-8" are 2 different ways of saying the
> same thing.

Right, except for the emphasis. But since we're talking about 
documentation, there's a difference between saying "parsing of non-UTF-8 
got slower" and "parsing of invalid UTF-8 got slower".

> However, note that various 7-bit encodings will be taken by such an
> interface as valid UTF-8, since all the bytes are 7-bit ASCII.
> There's nothing we can do about that, just something to keep in mind
> (it isn't supposed to happen in correct usage).

IIUC, in the case the "proper" UTF-8 decoder would do exactly the same 
thing in this case as the "fast path" upon discussion.

BTW, this is a case one could call both "non-UTF-8" and "valid UTF-8".

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: p.stephani2@gmail.com, sebastien@chapu.is, yyoncho@gmail.com,
>  31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 24 Apr 2019 09:57:26 +0300
> 
> > However, note that various 7-bit encodings will be taken by such an
> > interface as valid UTF-8, since all the bytes are 7-bit ASCII.
> > There's nothing we can do about that, just something to keep in mind
> > (it isn't supposed to happen in correct usage).
> 
> IIUC, in the case the "proper" UTF-8 decoder would do exactly the same 
> thing in this case as the "fast path" upon discussion.
> 
> BTW, this is a case one could call both "non-UTF-8" and "valid UTF-8".

No, it's valid UTF-8.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 24.04.2019 10:28, Eli Zaretskii wrote:

>> BTW, this is a case one could call both "non-UTF-8" and "valid UTF-8".
> 
> No, it's valid UTF-8.

That's what I said.

Anyway, this is just a discussion about words. Please feel free to 
disregard.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 23.04.2019 17:58, Eli Zaretskii wrote:

>> Doing that for buffer text as well might be helpful.
> 
> In what use cases would this be helpful?  Most cases of decoding text
> in a buffer happen when we read text from files, where we already have
> an internal optimization for plain ASCII files.

HTTP request response buffers? They can be as large as several 
megabytes, as we have established in this discussion.

> We could perhaps try
> a similar optimization for UTF-8 instead of just ASCII.

I think so.

> Use cases where we read without decoding and then decode buffer
> contents "by hand" are relatively rare, certainly when the stuff to
> decode is so large that the performance gains will be tangible.

Maybe be. We could try and benchmark, though.

>> So that's why I mentioned decode-coding-string (though
>> code_convert_string would be a better choice; or decode_coding_object?),
>> as opposed to creating a new specialized function.
> 
> code_convert_string also handles encoding, though.

That's just one more comparison. We could also do that in 
decode_coding_object instead, but I'm not sure about the overhead of the 
intervening code.

>> What I can understand from our testing, this kind of change improves
>> performance for all kinds of strings when the source encoding is
>> utf_8_unix. Even for large ones (despite you expecting otherwise).
> 
> I tested 10K strings, and the advantage there already becomes
> relatively small.  10K characters may be a lot for strings, but it
> isn't for buffers.

That's probably true. I have tried a similar shortcut, removing the 
code_convert_string call in json_encode (which is called once from 
json-parse-string), and that did not measurably affect its performance.

But it did increase the performance of json-serialize, by more than 2x, 
on the same test data I've been using.

Like 3.8s to 1.6s for 10 iterations.

Can we do something like that? Removing conversion altogether is 
probably not an option, but even using Fstring_as_unibyte instead lead 
to a significant improvement (2,43s with this approach).

diff --git a/src/json.c b/src/json.c
index 7d6d531427..01682473ca 100644
--- a/src/json.c
+++ b/src/json.c
@@ -266,7 +266,7 @@ json_encode (Lisp_Object string)
  {
    /* FIXME: Raise an error if STRING is not a scalar value
       sequence.  */
-  return code_convert_string (string, Qutf_8_unix, Qt, true, true, true);
+  return string;
  }

  static AVOID

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Cc: sebastien@chapu.is, yyoncho@gmail.com, 31138@debbugs.gnu.org
> Date: Wed, 24 Apr 2019 18:55:45 +0300
> 
> On 23.04.2019 17:58, Eli Zaretskii wrote:
> 
> >> Doing that for buffer text as well might be helpful.
> > 
> > In what use cases would this be helpful?  Most cases of decoding text
> > in a buffer happen when we read text from files, where we already have
> > an internal optimization for plain ASCII files.
> 
> HTTP request response buffers? They can be as large as several 
> megabytes, as we have established in this discussion.

Please point me to examples of this, I don't think I understand what
you have in mind.

> > We could perhaps try
> > a similar optimization for UTF-8 instead of just ASCII.
> 
> I think so.

Turns out we already do, I just missed that.

> But it did increase the performance of json-serialize, by more than 2x, 
> on the same test data I've been using.
> 
> Like 3.8s to 1.6s for 10 iterations.
> 
> Can we do something like that? Removing conversion altogether is 
> probably not an option, but even using Fstring_as_unibyte instead lead 
> to a significant improvement (2,43s with this approach).

What does libjansson do if it receives strings with bytes it cannot
interpret?  Like raw bytes or UTF-8 sequences whose length is more
than 4 bytes?

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 24.04.2019 19:21, Eli Zaretskii wrote:

> Please point me to examples of this, I don't think I understand what
> you have in mind.

Buffers with output from a process. In this example, the process is a 
network connection. Not sure it applies, though. Since output arrives 
piece-by-piece, maybe it gets decoded with *-string routines 
(mm-decode-string uses that).

>>> We could perhaps try
>>> a similar optimization for UTF-8 instead of just ASCII.
>>
>> I think so.
> 
> Turns out we already do, I just missed that.

OK.

> What does libjansson do if it receives strings with bytes it cannot
> interpret?  Like raw bytes or UTF-8 sequences whose length is more
> than 4 bytes?

One could test that directly, but its API docs list different kinds of 
errors, including:

   json_error_invalid_utf8
   json_error_null_character
   json_error_null_byte_in_key

And we check for error status in functions that use the encoded values.

Reading the sources, there are several places where the first error is 
signaled.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: sebastien@chapu.is, yyoncho@gmail.com, 31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 24 Apr 2019 19:46:16 +0300
> 
> On 24.04.2019 19:21, Eli Zaretskii wrote:
> 
> > Please point me to examples of this, I don't think I understand what
> > you have in mind.
> 
> Buffers with output from a process. In this example, the process is a 
> network connection. Not sure it applies, though. Since output arrives 
> piece-by-piece, maybe it gets decoded with *-string routines 
> (mm-decode-string uses that).

No, I think it's decoded by low-level code which reads output from the
process.  At least by default.

> > What does libjansson do if it receives strings with bytes it cannot
> > interpret?  Like raw bytes or UTF-8 sequences whose length is more
> > than 4 bytes?
> 
> One could test that directly, but its API docs list different kinds of 
> errors, including:
> 
>    json_error_invalid_utf8
>    json_error_null_character
>    json_error_null_byte_in_key
> 
> And we check for error status in functions that use the encoded values.
> 
> Reading the sources, there are several places where the first error is 
> signaled.

The question is, do we want to signal an error ourselves, or do we
want to rely on the library?

(And I need to refresh my memory regarding the differences between the
internal representation of text and strict UTF-8.)

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 24.04.2019 20:06, Eli Zaretskii wrote:

> No, I think it's decoded by low-level code which reads output from the
> process.  At least by default.

But what conversion primitives does that low-level code use? 
code_convert_region?

> The question is, do we want to signal an error ourselves, or do we
> want to rely on the library?

Relying on the library is probably faster. Though I'm not sure why we 
would be confident enough to rely on it in this case, but not the other.

I'll happily benchmark a patch, if you have any.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: sebastien@chapu.is, yyoncho@gmail.com, 31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 24 Apr 2019 20:36:56 +0300
> 
> On 24.04.2019 20:06, Eli Zaretskii wrote:
> 
> > No, I think it's decoded by low-level code which reads output from the
> > process.  At least by default.
> 
> But what conversion primitives does that low-level code use? 
> code_convert_region?

AFAICS, it uses decode_coding_c_string.  Incoming data doesn't have to
be UTF-8, mind you, it could be anything, including an unknown
encoding that needs detecting, and only the detection tells us it's
UTF-8.

> > The question is, do we want to signal an error ourselves, or do we
> > want to rely on the library?
> 
> Relying on the library is probably faster.

Why faster?

And more importantly, will the error indication be useful enough to
the Lisp program that triggered it?  If we signal an error, we can
make sure of that.

> I'll happily benchmark a patch, if you have any.

OK.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 24.04.2019 20:43, Eli Zaretskii wrote:

>> But what conversion primitives does that low-level code use?
>> code_convert_region?
> 
> AFAICS, it uses decode_coding_c_string.  Incoming data doesn't have to
> be UTF-8, mind you, it could be anything, including an unknown
> encoding that needs detecting, and only the detection tells us it's
> UTF-8.

All right, thank you. If I ever have anything solid to propose, I will 
do so later.

>> Relying on the library is probably faster.
> 
> Why faster?

Because the library will perform its validation anyway, during normal 
execution. Any extra validation work Emacs performs, will take time on 
top of that (of course, I don't know how much time).

> And more importantly, will the error indication be useful enough to
> the Lisp program that triggered it?  If we signal an error, we can
> make sure of that.

We still signal errors if the library returns an error code.

More importantly, we also perform additional validation ourselves if 
something goes wrong. See the pieces of code like

       if (json == NULL)
         {
           /* A failure can be caused either by an invalid string or by
              low memory.  */
           json_check_utf8 (encoded);
           json_out_of_memory ();
         }

Anyway, speeding up encoding is not as important as it was with 
decoding, because at least in this case Lisp programs can control how 
much data is sent. So let's not spend too much time and effort on it.

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> Cc: sebastien@chapu.is, yyoncho@gmail.com, 31138@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 24 Apr 2019 23:25:32 +0300
> 
> >> Relying on the library is probably faster.
> > 
> > Why faster?
> 
> Because the library will perform its validation anyway, during normal 
> execution.

But if we validate the string before even calling the library, that
will be even faster, no?

> Any extra validation work Emacs performs, will take time on 
> top of that (of course, I don't know how much time).

Didn't we just establish that it's almost negligible?  Especially for
short strings?

> > And more importantly, will the error indication be useful enough to
> > the Lisp program that triggered it?  If we signal an error, we can
> > make sure of that.
> 
> We still signal errors if the library returns an error code.

But we need to guess the reason.

> Anyway, speeding up encoding is not as important as it was with 
> decoding, because at least in this case Lisp programs can control how 
> much data is sent. So let's not spend too much time and effort on it.

OK.

bug#31138: Native json slower than json.el

[Dmitry Gutov]

On 25.04.2019 13:44, Eli Zaretskii wrote:

>>> Why faster?
>>
>> Because the library will perform its validation anyway, during normal
>> execution.
> 
> But if we validate the string before even calling the library, that
> will be even faster, no?

Only if the string is invalid. There's not much point in optimizing for 
that situation, I think.

>> Any extra validation work Emacs performs, will take time on
>> top of that (of course, I don't know how much time).
> 
> Didn't we just establish that it's almost negligible?  Especially for
> short strings?

Probably. But I can't say for sure without seeing the code.

>>> And more importantly, will the error indication be useful enough to
>>> the Lisp program that triggered it?  If we signal an error, we can
>>> make sure of that.
>>
>> We still signal errors if the library returns an error code.
> 
> But we need to guess the reason.

I think I've addressed that in the part of the reply you cut out: "we 
also perform additional validation ourselves if something goes wrong". 
That can be extended, if you like.

bug#31138: Native json slower than json.el

[Lars Ingebrigtsen]

This was a very long thread, but it looks like the performance issues
were fixed by:

commit c26556bd18f8ca1e891bd1750c9f95b21ea457b0
Author:     Eli Zaretskii <eliz@gnu.org>
AuthorDate: Sat Nov 23 11:27:43 2019 +0200

    Fix and speed up en/decoding of UTF-8 strings
    
So is there anything more to be done in this bug report?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#31138: Native json slower than json.el

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Eli Zaretskii <eliz@gnu.org>,  sebastien@chapu.is,  yyoncho@gmail.com,
>   31138@debbugs.gnu.org
> Date: Sun, 23 Aug 2020 01:28:36 +0200
> 
> This was a very long thread, but it looks like the performance issues
> were fixed by:
> 
> commit c26556bd18f8ca1e891bd1750c9f95b21ea457b0
> Author:     Eli Zaretskii <eliz@gnu.org>
> AuthorDate: Sat Nov 23 11:27:43 2019 +0200
> 
>     Fix and speed up en/decoding of UTF-8 strings
>     
> So is there anything more to be done in this bug report?

I think this can be closed, so I've now done so.

Thanks.