public inbox for [email protected]  
help / color / mirror / Atom feed
From: Andrew Dunstan <[email protected]>
To: Jacob Champion <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: WIP Incremental JSON Parser
Date: Tue, 2 Apr 2024 17:12:51 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAOYmi+mQvywAyFZTfujatvY3u2PbzVeqotaWSdB4ipRndB3FbA@mail.gmail.com>
References: <[email protected]>
	<CAOYmi+kgckFsBNOBsrekDipq88G56pVndVXKcA7qMWYx9feChQ@mail.gmail.com>
	<CAD5tBcLjFnTnAvUpQg-K5sJa3ECnEj8CQZ-AYrH1k+F=42D+Vg@mail.gmail.com>
	<CAOYmi+n1fxCt3QGz9g=GfY1+J6__vVqsAud=HYwpUrfZM_VJHw@mail.gmail.com>
	<CAD5tBcLi2ffZkktV2qrsKSBykE-N8CiYgrfbv0vZ-F7=xLFeqw@mail.gmail.com>
	<CAD5tBcJg-Ja3CcY8N+NtziFN8TL8JRWv97s=-siT=Oep2+JGjw@mail.gmail.com>
	<CAOYmi+nY=rF6dJCzaOuA3d-3FbwXCcecOs_S1NutexFA3dRXAw@mail.gmail.com>
	<CAOYmi+m8yeRLFPj68e0zOfZGKm1CAmWwrfsEmCjYxPTWn7itnA@mail.gmail.com>
	<CAD5tBcLm=+LUrGZftmRGxq1tWE9eAcviVJgEF=AX8P_=NDd=eg@mail.gmail.com>
	<CAOYmi+nEs4wVLSU4=sZU1Mf7j-bmsQoYD7CMZoEhwtNvz1jTOQ@mail.gmail.com>
	<CAD5tBcK-RzoWpwb2UBHFOoMxrn1LHaKGt_XJd9wKiQg=rwBjNA@mail.gmail.com>
	<[email protected]>
	<CAOYmi+nGbv3pGgGeWG7xTAnYo=gPXkeJ-sBT+kvZS1kfRF7H1Q@mail.gmail.com>
	<[email protected]>
	<CAOYmi+mQvywAyFZTfujatvY3u2PbzVeqotaWSdB4ipRndB3FbA@mail.gmail.com>


On 2024-04-02 Tu 15:38, Jacob Champion wrote:
> On Mon, Apr 1, 2024 at 4:53 PM Andrew Dunstan <[email protected]> wrote:
>> Anyway, here are new patches. I've rolled the new semantic test into the
>> first patch.
> Looks good! I've marked RfC.


Thanks! I appreciate all the work you've done on this. I will give it 
one more pass and commit RSN.

>
>> json_lex() is not really a very hot piece of code.
> Sure, but I figure if someone is trying to get the performance of the
> incremental parser to match the recursive one, so we can eventually
> replace it, it might get a little warmer. :)

I don't think this is where the performance penalty lies. Rather, I 
suspect it's the stack operations in the non-recursive parser itself. 
The speed test doesn't involve any partial token processing at all, and 
yet the non-recursive parser is significantly slower in that test.

>>> I think it'd be good for a v1.x of this feature to focus on
>>> simplification of the code, and hopefully consolidate and/or eliminate
>>> some of the duplicated parsing work so that the mental model isn't
>>> quite so big.
>> I'm not sure how you think that can be done.
> I think we'd need to teach the lower levels of the lexer about
> incremental parsing too, so that we don't have two separate sources of
> truth about what ends a token. Bonus points if we could keep the parse
> state across chunks to the extent that we didn't need to restart at
> the beginning of the token every time. (Our current tools for this are
> kind of poor, like the restartable state machine in PQconnectPoll.
> While I'm dreaming, I'd like coroutines.) Now, whether the end result
> would be more or less maintainable is left as an exercise...
>

I tried to disturb the main lexer processing as little as possible. We 
could possibly unify the two paths, but I have a strong suspicion that 
would result in a performance hit (the main part of the lexer doesn't 
copy any characters at all, it just keeps track of pointers into the 
input). And while the code might not undergo lots of change, the routine 
itself is quite performance critical.

Anyway, I think that's all something for another day.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com







view thread (44+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: WIP Incremental JSON Parser
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox