Re: Reminder: Review for Zach's parsing library starts on Feb 19th

From: Zach Laine (whatwasthataddress_at_[hidden])
Date: 2024-03-01 03:25:20

On Thu, Feb 29, 2024 at 9:07 PM Zach Laine <whatwasthataddress_at_[hidden]> wrote:
>
> On Thu, Feb 29, 2024 at 8:44 PM Peter Dimov via Boost
> <boost_at_[hidden]> wrote:
> >
> > Zach Laine wrote:
> > > I'm shocked too.  That's really crazy.  Believe me, I'm not interested in merging
> > > something with orders of magnitude of perf overhead into Boost.  I will be
> > > getting to the bottom of this long before a merge could take place.
> >
> > My profiler (VS2022) says that the top performance problem is the construction
> > of a stringstream here
> >
> > https://github.com/tzlaine/parser/blob/f99ae3b94ad0acef0cc92166d5108aade41da4ea/include/boost/parser/detail/printing.hpp#L624
> >
> > This constructs a std::locale, which is apparently very slow.
> >
> > When I fix it
> >
> > diff --git a/include/boost/parser/detail/printing.hpp b/include/boost/parser/detail/printing.hpp
> > index 1e204796..6cbec059 100644
> > --- a/include/boost/parser/detail/printing.hpp
> > +++ b/include/boost/parser/detail/printing.hpp
> > @@ -621,11 +621,19 @@ namespace boost { namespace parser { namespace detail {
> >          flags f,
> >          Attribute const & attr)
> >      {
> > -        std::stringstream oss;
> >          if (detail::do_trace(f))
> > +        {
> > +            std::stringstream oss;
> >              detail::print_parser(context, parser, oss);
> > -        return scoped_trace_t<Iter, Sentinel, Context, Attribute>(
> > -            first, last, context, f, attr, oss.str());
> > +
> > +            return scoped_trace_t<Iter, Sentinel, Context, Attribute>(
> > +                first, last, context, f, attr, oss.str());
> > +        }
> > +        else
> > +        {
> > +            return scoped_trace_t<Iter, Sentinel, Context, Attribute>(
> > +                first, last, context, f, attr, {});
> > +        }
> >      }
> >
> >      template<typename Context, typename Attribute>
> >
> > the top function becomes the constructor of scoped_trace_t. (It's probably
> > not getting inlined.)
>
> Thanks.  I fixed it as well, and now I get about 1.5-2X worse
> performance than Boost.JSON.  I'm just building the "pretty" example
> from Boost.JSON, and "json" and "callback_json" from my examples.
> When I feed these the file at
> https://world.openfoodfacts.org/api/v0/product/5060292302201.json ,
> and prepend "time", I get these results:
>
> pretty (Boost.JSON):
> real    0m0.014s
> user    0m0.004s
> sys     0m0.000s
>
> json (Parser):
> real    0m0.022s
> user    0m0.020s
> sys     0m0.000s
>
> callback_json (Parser, no JSON object creation):
> real    0m0.023s
> user    0m0.018s
> sys     0m0.004s
>
> I'm not sure what the tests reported earlier were doing, but I
> definitely don't see orders of magnitude difference.
>
> > Looks like the tracing functionality costs a lot even when off.
>
> If I can verify this (my fix might be a little different), I'll turn
> the runtime flag into a template parameter.  That should do it.

Should have mentioned: https://github.com/tzlaine/parser/issues/152

Zach