bug#40693: 28.0.50; json-encode-alist changes alist

[Basil L. Contovounesios]

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: darthandrus@gmail.com, 40693@debbugs.gnu.org
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Date: Wed, 29 Apr 2020 15:08:57 +0300
>> 
>> > No one tried to come up with arguments why this has to be in emacs-27.
>> 
>> Let me try:
>> 
>> It fixes a bug, one which could be annoying to investigate, the fix is 
>> small and localized to the case when json-encoding-object-sort-predicate 
>> is non-nil (so pretty safe).
>
> It also makes the function slower.  Which may be an important issue
> for JSON processing.  Callers that don't care about the original list
> will be "punished" regardless.
>
> How about adding an optional argument instead, by default off, to
> request this behavior?  then callers who care about the original alist
> could request a non-destructive operation, and others won't suffer any
> slowdown.

That was my first thought too.  I have a local WIP branch where I am
refactoring parts of json.el to improve performance, and avoiding the
proposed copy-sequence is one of the fixes.  I've also found a couple of
test JSON files to benchmark against.

>> It's not a regression from Emacs 26, though.
>
> Right.

The reasons I thought the copy-sequence fix might be desirable in Emacs
27 are:

1. It's a simple enough fix to go into the release branch.

2. Users who enable json-encoding-object-sort-predicate are already
   trading off performance for sort order.  In addition to the cost of
   sorting, hash table and plist objects have to first be converted to
   alists before they can be sorted.  My guess is that an extra
   copy-sequence won't make a big difference here.

3. Users who care about sheer performance of JSON serialisation will
   either avoid json-encoding-object-sort-predicate or use the newer
   Jansson functions.

I don't personally mind whether this gets into emacs-27; I just wanted
to see what others thought.

Thanks,

-- 
Basil