Horton

Thread

Commits

  1. Fix transient memory leakage in jsonpath evaluation.

  1. Avoiding memory leakage in jsonpath evaluation

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-03-17T21:33:31Z

    I got an off-list report that a query like this consumes
an unreasonable amount of memory:

SELECT jsonb_path_query((SELECT jsonb_agg(i) FROM generate_series(1,10000) i),
                        '$[*] ? (@ < $)');

For me, that eats about 6GB by the time it's done executing.
If that doesn't seem like a lot to you, just add another zero to the
generate_series call, and then it'll be more like 600GB, because the
leakage is O(N^2).

Admittedly, this isn't an especially useful query: its runtime is
also O(N^2), because that path expression basically requires us to
compare every element of the input JSON array to every other element.
But it's not cool that it leaks so much memory while at it.

I poked into this and found that the leakage is entirely composed of
"JsonValueList"s that are built during path evaluation and then just
left to rot until the end of jsonb_path_query().  We can fix it by
being careful to free those lists on the way out of each jsonpath
evaluation function that creates one.  However, just doing that would
mean adding pfree overhead on top of palloc overhead, so I went a bit
further and reimplemented JsonValueList to be more compact and cheaper
to allocate/free.  The attached seems to be a bit faster than the
existing code as well as not leaking so much memory.  See the draft
commit message for more details.

			regards, tom lane

  2. Re: Avoiding memory leakage in jsonpath evaluation

    Chao Li <li.evan.chao@gmail.com> — 2026-03-18T07:52:40Z

    
> On Mar 18, 2026, at 05:33, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> I got an off-list report that a query like this consumes
> an unreasonable amount of memory:
> 
> SELECT jsonb_path_query((SELECT jsonb_agg(i) FROM generate_series(1,10000) i),
>                        '$[*] ? (@ < $)');
> 
> For me, that eats about 6GB by the time it's done executing.
> If that doesn't seem like a lot to you, just add another zero to the
> generate_series call, and then it'll be more like 600GB, because the
> leakage is O(N^2).
> 
> Admittedly, this isn't an especially useful query: its runtime is
> also O(N^2), because that path expression basically requires us to
> compare every element of the input JSON array to every other element.
> But it's not cool that it leaks so much memory while at it.
> 
> I poked into this and found that the leakage is entirely composed of
> "JsonValueList"s that are built during path evaluation and then just
> left to rot until the end of jsonb_path_query().  We can fix it by
> being careful to free those lists on the way out of each jsonpath
> evaluation function that creates one.  However, just doing that would
> mean adding pfree overhead on top of palloc overhead, so I went a bit
> further and reimplemented JsonValueList to be more compact and cheaper
> to allocate/free.  The attached seems to be a bit faster than the
> existing code as well as not leaking so much memory.  See the draft
> commit message for more details.
> 
> regards, tom lane

This patch looks like a big win. It not only saves memory, but also makes the query much faster.

I tested the query on my MacBook M4, increasing the iteration count from 10000 to 50000.

Current master (3b4c2b9db25):
```
evantest=# SELECT jsonb_path_query((SELECT jsonb_agg(i) FROM generate_series(1,50000) i), '$[*] ? (@ < $)');
Time: 208581.771 ms (03:28.582)
evantest=# SELECT jsonb_path_query((SELECT jsonb_agg(i) FROM generate_series(1,50000) i), '$[*] ? (@ < $)');
Time: 217269.595 ms (03:37.270)
```

With the patch:
```
evantest=# SELECT jsonb_path_query((SELECT jsonb_agg(i) FROM generate_series(1,50000) i), '$[*] ? (@ < $)');
Time: 18674.580 ms (00:18.675)
evantest=# SELECT jsonb_path_query((SELECT jsonb_agg(i) FROM generate_series(1,50000) i), '$[*] ? (@ < $)');
Time: 18889.329 ms (00:18.889)
```

My observations were:

* Before the patch, the backend process memory usage fluctuated between roughly 50GB and 145GB, while CPU usage stayed around 30%.
* With the patch, the backend process memory usage stayed stable at around 30MB, while CPU usage stayed around 100%.

After reviewing the patch, I thought JsonValueListLength() might be worth optimizing, since it is O(n). I tried adding an ntotal_items field to JsonValueList to track the total number of items, similar to the last pointer that is only meaningful in the base chunk. But that did not help in my test, and I realized JsonValueListLength() is not on the hottest path, so I dropped that idea.

From the MacOS Instruments tool, the most expensive parts seem to be fillJsonbValue, JsonbIteratorNext, cmp_var_common, and cmp_numerics. But those look like separate topics.

Overall, this looks like a solid patch.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

  3. Re: Avoiding memory leakage in jsonpath evaluation

    Chao Li <li.evan.chao@gmail.com> — 2026-03-18T07:57:53Z

    
> On Mar 18, 2026, at 15:52, Chao Li <li.evan.chao@gmail.com> wrote:
> 
> 
> 
>> On Mar 18, 2026, at 05:33, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> 
>> I got an off-list report that a query like this consumes
>> an unreasonable amount of memory:
>> 
>> SELECT jsonb_path_query((SELECT jsonb_agg(i) FROM generate_series(1,10000) i),
>>                       '$[*] ? (@ < $)');
>> 
>> For me, that eats about 6GB by the time it's done executing.
>> If that doesn't seem like a lot to you, just add another zero to the
>> generate_series call, and then it'll be more like 600GB, because the
>> leakage is O(N^2).
>> 
>> Admittedly, this isn't an especially useful query: its runtime is
>> also O(N^2), because that path expression basically requires us to
>> compare every element of the input JSON array to every other element.
>> But it's not cool that it leaks so much memory while at it.
>> 
>> I poked into this and found that the leakage is entirely composed of
>> "JsonValueList"s that are built during path evaluation and then just
>> left to rot until the end of jsonb_path_query().  We can fix it by
>> being careful to free those lists on the way out of each jsonpath
>> evaluation function that creates one.  However, just doing that would
>> mean adding pfree overhead on top of palloc overhead, so I went a bit
>> further and reimplemented JsonValueList to be more compact and cheaper
>> to allocate/free.  The attached seems to be a bit faster than the
>> existing code as well as not leaking so much memory.  See the draft
>> commit message for more details.
>> 
>> regards, tom lane
> 
> This patch looks like a big win. It not only saves memory, but also makes the query much faster.
> 
> I tested the query on my MacBook M4, increasing the iteration count from 10000 to 50000.
> 
> Current master (3b4c2b9db25):
> ```
> evantest=# SELECT jsonb_path_query((SELECT jsonb_agg(i) FROM generate_series(1,50000) i), '$[*] ? (@ < $)');
> Time: 208581.771 ms (03:28.582)
> evantest=# SELECT jsonb_path_query((SELECT jsonb_agg(i) FROM generate_series(1,50000) i), '$[*] ? (@ < $)');
> Time: 217269.595 ms (03:37.270)
> ```
> 
> With the patch:
> ```
> evantest=# SELECT jsonb_path_query((SELECT jsonb_agg(i) FROM generate_series(1,50000) i), '$[*] ? (@ < $)');
> Time: 18674.580 ms (00:18.675)
> evantest=# SELECT jsonb_path_query((SELECT jsonb_agg(i) FROM generate_series(1,50000) i), '$[*] ? (@ < $)');
> Time: 18889.329 ms (00:18.889)
> ```
> 
> My observations were:
> 
> * Before the patch, the backend process memory usage fluctuated between roughly 50GB and 145GB, while CPU usage stayed around 30%.
> * With the patch, the backend process memory usage stayed stable at around 30MB, while CPU usage stayed around 100%.
> 
> After reviewing the patch, I thought JsonValueListLength() might be worth optimizing, since it is O(n). I tried adding an ntotal_items field to JsonValueList to track the total number of items, similar to the last pointer that is only meaningful in the base chunk. But that did not help in my test, and I realized JsonValueListLength() is not on the hottest path, so I dropped that idea.
> 
> From the MacOS Instruments tool, the most expensive parts seem to be fillJsonbValue, JsonbIteratorNext, cmp_var_common, and cmp_numerics. But those look like separate topics.
> 
> Overall, this looks like a solid patch.
> 

Forgot to mention that, to run the tests, I turned off debug and assertion, and compiled with -O2.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

  4. Re: Avoiding memory leakage in jsonpath evaluation

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-03-18T16:01:11Z

    Chao Li <li.evan.chao@gmail.com> writes:
>> On Mar 18, 2026, at 05:33, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I got an off-list report that a query like this consumes
>> an unreasonable amount of memory:
>> SELECT jsonb_path_query((SELECT jsonb_agg(i) FROM generate_series(1,10000) i),
>> '$[*] ? (@ < $)');

> This patch looks like a big win. It not only saves memory, but also makes the query much faster.

Thanks for looking at it!

> I tested the query on my MacBook M4, increasing the iteration count from 10000 to 50000.

Hmm, at that point you were probably mostly measuring how fast the
machine could swap :-(.  I had been testing on Linux x86_64, but when
I tried it on macOS just now, the unpatched memory consumption seemed
even worse than on Linux.

> After reviewing the patch, I thought JsonValueListLength() might be
> worth optimizing, since it is O(n).

Well, it's really O(log N) given that we doubled the chunk size each
time we added a chunk.  I had also thought about tracking the total
number of items, but concluded that incrementing an additional counter
during each item addition couldn't possibly get repaid given that
amortization and the fact that we don't compute the list length more
than once in any operation.

However, looking at it again today, I realized that no caller actually
needs to know the total length of a long JsonValueList.  They only
really care whether the list has zero, one, or more than one member.
And we can determine that just by looking at the base chunk.

Hence, v2 attached.  0001 is the same as before, and 0002 removes
JsonValueListLength in favor of some constant-time tests.  (I'd
plan to squash these to one commit in the end, but it seemed easier
to review if I present the delta-from-yesterday separately.)

BTW, I don't love the function name JsonValueListIsMultiple, but if
there's a common term analogous to "singleton" but describing sets
with more than one member, I don't know it.

			regards, tom lane

  5. Re: Avoiding memory leakage in jsonpath evaluation

    Chao Li <li.evan.chao@gmail.com> — 2026-03-19T02:27:50Z

    
> On Mar 19, 2026, at 00:01, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> Chao Li <li.evan.chao@gmail.com> writes:
>>> On Mar 18, 2026, at 05:33, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> I got an off-list report that a query like this consumes
>>> an unreasonable amount of memory:
>>> SELECT jsonb_path_query((SELECT jsonb_agg(i) FROM generate_series(1,10000) i),
>>> '$[*] ? (@ < $)');
> 
>> This patch looks like a big win. It not only saves memory, but also makes the query much faster.
> 
> Thanks for looking at it!
> 
>> I tested the query on my MacBook M4, increasing the iteration count from 10000 to 50000.
> 
> Hmm, at that point you were probably mostly measuring how fast the
> machine could swap :-(.  I had been testing on Linux x86_64, but when
> I tried it on macOS just now, the unpatched memory consumption seemed
> even worse than on Linux.

My MacBook has only 32GB of RAM, while the query needed far more than that, so the system spent a lot of time swapping and the CPUs could not run at full speed. That really highlights how valuable the memory savings from this patch are.

Actually, I also tested with 10000, where memory pressure was not severe enough to overwhelm RAM, and the patched version was still faster there.

On master:
```
evantest=# SELECT jsonb_path_query((SELECT jsonb_agg(i) FROM generate_series(1,10000) i), '$[*] ? (@ < $)');
Time: 1272.900 ms (00:01.273)
evantest=# SELECT jsonb_path_query((SELECT jsonb_agg(i) FROM generate_series(1,10000) i), '$[*] ? (@ < $)');
Time: 1265.017 ms (00:01.265)
evantest=# SELECT jsonb_path_query((SELECT jsonb_agg(i) FROM generate_series(1,10000) i), '$[*] ? (@ < $)');
Time: 1160.833 ms (00:01.161)
```

Patched:
```
evantest=# SELECT jsonb_path_query((SELECT jsonb_agg(i) FROM generate_series(1,10000) i), '$[*] ? (@ < $)');
Time: 794.270 ms
evantest=# SELECT jsonb_path_query((SELECT jsonb_agg(i) FROM generate_series(1,10000) i), '$[*] ? (@ < $)');
Time: 779.556 ms
evantest=# SELECT jsonb_path_query((SELECT jsonb_agg(i) FROM generate_series(1,10000) i), '$[*] ? (@ < $)');
Time: 769.634 ms
```

> 
>> After reviewing the patch, I thought JsonValueListLength() might be
>> worth optimizing, since it is O(n).
> 
> Well, it's really O(log N) given that we doubled the chunk size each
> time we added a chunk.  I had also thought about tracking the total
> number of items, but concluded that incrementing an additional counter
> during each item addition couldn't possibly get repaid given that
> amortization and the fact that we don't compute the list length more
> than once in any operation.

When I said O(n), I was treating “n” as the length of the chunk list, whereas you were treating “n” as the total number of values. Yes, I agree your interpretation makes more sense here.

> 
> However, looking at it again today, I realized that no caller actually
> needs to know the total length of a long JsonValueList.  They only
> really care whether the list has zero, one, or more than one member.
> And we can determine that just by looking at the base chunk.

I had noticed that the total count wasn’t really used either, but since you added it, I assumed you had some possible future use for it, so I didn’t mention it.

> 
> Hence, v2 attached.  0001 is the same as before, and 0002 removes
> JsonValueListLength in favor of some constant-time tests.  (I'd
> plan to squash these to one commit in the end, but it seemed easier
> to review if I present the delta-from-yesterday separately.)
> 
> BTW, I don't love the function name JsonValueListIsMultiple, but if
> there's a common term analogous to "singleton" but describing sets
> with more than one member, I don't know it.
> 

Maybe JsonValueListHasMultiple?

If it were me, I might name them like this:

* JsonValueListIsEmpty
* JsonValueListHasOneItem
* JsonValueListHasMultipleItems

Those may not look as elegant, but at least to me, as a non-English speaker, they feel more direct and easier to understand.

One nitpick on 0002 is that, these three functions don’t modify jvl, so the parameter could be made const.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

  6. Re: Avoiding memory leakage in jsonpath evaluation

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-03-19T15:47:29Z

    Chao Li <li.evan.chao@gmail.com> writes:
> On Mar 19, 2026, at 00:01, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> BTW, I don't love the function name JsonValueListIsMultiple, but if
>> there's a common term analogous to "singleton" but describing sets
>> with more than one member, I don't know it.

> Maybe JsonValueListHasMultiple?
> If it were me, I might name them like this:
> * JsonValueListIsEmpty
> * JsonValueListHasOneItem
> * JsonValueListHasMultipleItems

I think JsonValueListIsSingleton is fine: it's a well-understood
term that appears in hundreds of other places in our tree.  I took
your suggestion of JsonValueListHasMultipleItems, though.

> One nitpick on 0002 is that, these three functions don’t modify jvl, so the parameter could be made const.

Yeah.  I'd left them like that because the pre-existing
JsonValueListIsEmpty() wasn't using const, but I agree it's
neater to do so.  Changed.

A related annoyance is that I had to remove "const" from the
JsonValueList parameters of JsonValueListInitIterator and
wrapItemsInArray.  That's because JsonValueListNext returns
not-const JsonbValue *, and my compiler complained (rightly)
that returning a pointer into the embedded items[] array would
be casting away const.  It's conceivable that we could change
JsonValueListNext to return const JsonbValue *.  But that would
require wholesale const-ification of a lot of calling code, and
I judged it not worth the trouble, or at least material for a
different patch.

Pushed with those changes.  Thanks for reviewing!

			regards, tom lane