Re: [review][json] My review

From: Gavin Lambert (boost_at_[hidden])
Date: 2020-09-23 13:43:16

At 00:22 24/09/2020, Vinnie Falco wrote:
 >On Wed, Sep 23, 2020 at 1:40 AM Gavin Lambert wrote:
 >> I've walked back on that one a little bit; I now think that 
it's
 >ok for
 >> basic_parser to do that (to retain a zero-allocation 
guarantee)
 >but in
 >> that case it should treat numbers the same way (which it
 >currently
 >> doesn't), and ideally there should be a standard
 >slightly-higher-level
 >> parser that has a similar interface but does preassemble the
 >values,
 >> eliminating the _part callbacks (for people who prefer ease of 

 >use over
 >> raw performance).
 >
 >I had that originally, it was called number_parser, it was a
 >public
 >interface, and it was one of the first things to go during our
 >3-month
 >journey of optimization. You can fish it out of the commit log 
if
 >you
 >care to see it.

I didn't fish out the code, but given the class name, that sounds 
like the complete opposite of what I was suggesting.

To restate again, my suggestion is:

1. Remove on_int64/uint64/double from basic_parser's handler type.
2. Add on_number(string_view, size_t) to basic_parser's handler 
type -- having exactly the same semantics as on_key/string and 
their _part companions.
3. Add custom_parser, which wraps a basic_parser but has internal 
(dynamically allocated if needed) storage for partial 
keys/strings/numbers so it never calls _part methods on its own 
handler type.  It still uses on_number(string_view) but note that 
this (and on_key/string) have no reason to pass a size_t any more, 
because it can never be any different from the size() of the 
string_view.
4. The int64/uint64/double parsing code that used to be in 
basic_parser should be moved to a dedicated helper class, in turn 
used by parser and *optionally* used by the user's handler 
implementation for basic_parser/custom_parser.

This design has the proper separation of concerns between the 
parser layers (not dictating a numeric storage format, and keeping 
number parsing separate from JSON structural parsing).  It should 
not negatively impact performance of parser, and should positively 
affect performance of basic/custom_parser consumers who didn't 
want the default conversion anyway.