Re: Minor optimization in json.c

[Eli Zaretskii]

> From: Philipp Stephani <address@hidden>
> Date: Wed, 3 Oct 2018 21:57:24 +0200
> Cc: address@hidden
> 
> Thanks, just 2 stylistic comments.

Thanks for reviewing the code.

> 1. Could you try to eliminate global mutable state (i.e. the 
> json_inserted_bytes global variable)? I know that
> with the current threading implementation access is properly synchronized, 
> but the global variable is still a
> code smell and unnecessarily couples details of the threading implementation 
> with the JSON adapter code. I
> think you should be able to move it into the json_insert_data or 
> json_buffer_and_size structures.

I did that, but it needed adding new members to 2 data structures, and
copying the values back and forth between them.  Not sure this is more
elegant.

> 2. Could you try factoring out the buffer management code into new functions 
> in insdel.c with a simple
> interface? The details on how to keep track of the buffer insertion status 
> aren't really related to JSON, and it
> would be better to keep json.c focused on providing the JSON adapter (single 
> responsibility principle). Maybe
> with an interface such as:
> void begin_insert_utf8_string (void);
> void insert_utf8_string (const char *chars, ptrdiff_t nbytes);
> void finish_insert_utf8_string (ptrdiff_t nbytes);

I didn't do this.  I don't like breaking code into functions that have
no meaning on their own, especially when there's only one caller.  (We
do something similar in insert-file-contents, but the structure of the
code and the details are sufficiently different that I decided not to
make a single implementation that would support both.)

I also don't agree with the argument about tracking the insertion
status not being related to JSON: we do similar stuff in many places,
and even json.c itself has similarly "unrelated" code, like the one
under CONSP (lisp) in lisp_to_json_toplevel_1.

The changes are now pushed to master.