Re: [boost] [review][json] My review
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.
On Wed, Sep 23, 2020 at 7:21 AM Gavin Lambert via Boost
given the class name, that sounds like the complete opposite of what I was suggesting. ... 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
Yes, you have described number_parser. Again. Thanks
participants (2)
-
Gavin Lambert
-
Vinnie Falco