You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@age.apache.org by "crdv7 (via GitHub)" <gi...@apache.org> on 2026/08/13 06:07:31 UTC
[PR] Bound per-row memory in writable clauses [age]
crdv7 opened a new pull request, #2517:
URL: https://github.com/apache/age/pull/2517
## Summary
`SET` and `REMOVE` currently create EState-owned tuple slots for every updated
entity and allocate transient update state in an executor-lifetime memory
context. These resources remain until executor shutdown, so a large writable
statement becomes progressively more expensive and retains memory in
proportion to the number of processed rows. The UPDATE and DELETE RLS paths
have the same per-row slot ownership problem, and `MERGE ... SET` shares the
property-update path.
This PR gives row-local slots and scratch a row-local lifetime while preserving
the executor- and statement-lifetime state that must survive across rows.
It does not change Cypher semantics, agtype serialization, entity lookup, or
the on-disk representation.
## Root cause
`apply_update_list()` creates a label-table write slot with
`ExecInitExtraTupleSlot()` for every updated entity. That API appends the slot
to `EState.es_tupleTable` and pins its `TupleDesc` resource until executor
shutdown. Temporary RLS slots in the UPDATE and DELETE paths use the same API.
Instrumentation shows that the tuple table grows once per updated row:
```text
10,000 rows: es_tupleTable 7 -> 10,007
100,000 rows: es_tupleTable 7 -> 100,007
```
`apply_update_list()` and `process_delete_list()` also run their direct
allocations and downstream agtype builders while the current allocation
context is executor-lifetime. Row-local arrays, hash tables, parse state,
temporary strings, entity wrappers, and serialized values therefore
accumulate until the statement ends.
### CPU profile
The following profile is Apache AGE master at `80141740`, running `SET` over
100,000 vertices with two properties. It was sampled with:
```text
perf record -F 199 -e cycles:u -g --call-graph dwarf,16384
```
`ResourceOwnerAddToHash()` accounts for 66.74% of sampled CPU and
`ResourceOwnerForget()` for 27.98%. Both stacks originate from the per-row
EState-owned slots described above.
<img width="1600" height="718" alt="write-memory-baseline-annotated-static" src="https://github.com/user-attachments/assets/deb4d06d-53de-4e28-ba9e-dbd47e4afab1" />
The same workload on the final patched commit has no
`ResourceOwnerAddToHash()` or `ResourceOwnerForget()` samples. CPU time is
distributed across the actual property rebuild and PostgreSQL tuple/index
update paths; reclaiming the preceding row's memory context accounts for
1.08% of the patched profile.
<img width="1600" height="734" alt="write-memory-patched-annotated-static" src="https://github.com/user-attachments/assets/f67a8140-b1c7-405b-93e5-cdc532a78848" />
## Implementation
- Use `MakeSingleTupleTableSlot()` for the per-entity SET/REMOVE write slot
and the per-row UPDATE/DELETE RLS slots, then release each with
`ExecDropSingleTupleTableSlot()` in the same row.
- Run update/delete scratch allocation in the writable CustomScan's
`ecxt_per_tuple_memory`, and restore the caller's memory context before
returning.
- Reset that ExprContext before processing the next `SET`, `REMOVE`, DELETE,
or eager MERGE input row.
- Keep MERGE's insert slot EState-owned because it is initialized once and
reused for the node lifetime.
- Keep MERGE's `created_paths_list` as statement-lifetime de-duplication state.
Properties for a newly retained path are copied to `es_query_cxt` before the
row context is reset. Duplicate paths do not incur that copy.
The per-row bound applies to expression and writable-clause scratch. MERGE's
`created_paths_list` still retains one entry per unique path created by the
statement, and its eager result buffer retains one tuple per result row. Both
are required across input rows and are released when the MERGE node ends.
An audit of all `ExecInitExtraTupleSlot()` call sites found three per-row
uses that require this change. MERGE insert slots, initialization helpers, and
executor-shutdown scans are created once for their intended lifetime and stay
unchanged.
## Performance
### Environment and method
- Baseline AGE: `801417404978823bd8732452c3f7959017584785`
(Apache master)
- Patched AGE: `1ebaf28617d669b25dcfb9b3a1c5545f879210b9`
- PostgreSQL 18.4, `--disable-debug --disable-cassert`, `CFLAGS=-O2 -g`
- Intel Xeon 6982P-C, 8 cores / 16 threads, 29 GiB RAM
- Local Unix-domain socket
- `shared_buffers=1GB`, `fsync=off`, `synchronous_commit=off`,
`full_page_writes=off`, `autovacuum=off`, `jit=off`
The two variants use separate databases bound to their exact `age.so` files.
Each writable workload shown below has five measured runs in AB/BA-interleaved
variant order. Fixture reset, `VACUUM ANALYZE`, and library selection are
outside the timed interval. Elapsed values are medians; CV is the sample
coefficient of variation. Memory is the maximum private mapping total sampled
from the backend's `/proc/<pid>/smaps_rollup` during each statement.
The workloads exercise different parts of the writable executor:
- `SET` new key rebuilds each entity's property map while inserting a property.
- `SET` existing key replaces a value already present in the property map.
- `REMOVE` rebuilds the property map while omitting one existing key.
- `SET += map` exercises whole-map merge rather than a single-property update.
- `DELETE` does not rebuild or serialize properties, so it isolates the
per-row slot and scratch-lifetime cost more directly.
### 100,000 vertices, two properties
| Workload | Master median (CV) | Patched median (CV) | Change | Speedup |
|---|---:|---:|---:|---:|
| `SET` new key | 12.876 s (0.11%) | 635.564 ms (1.46%) | -95.06% | 20.26x |
| `SET` existing key | 12.880 s (0.30%) | 635.340 ms (0.21%) | -95.07% | 20.27x |
| `REMOVE` | 12.869 s (0.20%) | 612.503 ms (1.72%) | -95.24% | 21.01x |
| `SET += map` | 12.673 s (1.04%) | 635.912 ms (1.50%) | -94.98% | 19.93x |
| `DELETE` | 483.930 ms (0.13%) | 387.853 ms (0.14%) | -19.85% | 1.25x |
Across the four property-update workloads, the median maximum private memory is
1.00-1.12 GiB on master and 23-26 MiB with the patch.
For `DELETE`, it falls from approximately 271 MiB to 22 MiB.
### 100,000 vertices, 100 properties
| Workload | Master median (CV) | Patched median (CV) | Change | Speedup |
|---|---:|---:|---:|---:|
| `SET` new key | 17.542 s (0.53%) | 2.756 s (1.22%) | -84.29% | 6.36x |
| `SET` existing key | 17.514 s (0.20%) | 2.733 s (0.70%) | -84.40% | 6.41x |
| `REMOVE` | 17.586 s (0.50%) | 2.705 s (0.97%) | -84.62% | 6.50x |
| `SET += map` | 16.392 s (0.16%) | 2.250 s (0.75%) | -86.27% | 7.29x |
Across the wide-property workloads, the median maximum private memory is
4.90-5.62 GiB on master and 0.33-0.34 GiB with the patch.
The narrow workload demonstrates the slot-lifetime cost directly. The
100-property workload also magnifies scratch retained in the wrong memory
context. Remaining width-dependent agtype serialization work is intentionally
outside this PR.
## Reproduction
For a quick timing comparison, run the attached SQL once against Apache master
and once against this branch:
```bash
psql -X -d <database> -v row_count=100000 \
-f repro_write_memory_lifetime.sql
```
The script builds the fixture outside the measured statements and uses a
transaction rollback after every workload so that each starts from the same
data.
For elapsed time plus backend RSS/private-memory sampling on Linux, use the
attached one-command runner for each installed build:
```bash
PGDATABASE=<database> TAG=master ROWS=100000 KEYS=2 \
./run_write_memory_benchmark.sh
PGDATABASE=<database> TAG=patched ROWS=100000 KEYS=2 \
./run_write_memory_benchmark.sh
```
Set `KEYS=100` for the wide-property case. The runner emits local results under
`write-memory-results/` and also covers `MERGE ... ON MATCH SET`; generated
result files do not need to be attached to the PR.
[repro_write_memory_lifetime.sql](https://github.com/user-attachments/files/31015151/repro_write_memory_lifetime.sql)
[run_write_memory_benchmark.sh](https://github.com/user-attachments/files/31015152/run_write_memory_benchmark.sh)
## Testing
- PostgreSQL 18.4 release build with `COPT=-Werror`: 43/43 regression tests
passed.
- PostgreSQL 18.4 ASan + cassert + `MEMORY_CONTEXT_CHECKING`: 43/43 regression
tests passed, including the new multi-row and repeated-path MERGE coverage.
## Suggested review order
1. `src/backend/executor/cypher_set.c`: standalone per-row slots and the
per-tuple allocation boundary.
2. `src/backend/executor/cypher_delete.c`: DELETE scratch and RLS slot
ownership.
3. `src/backend/executor/cypher_merge.c`: per-row reset points and preservation
of cross-row de-duplication properties.
4. `regress/sql/cypher_merge.sql` and expected output.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Bound per-row memory in writable clauses [age]
Posted by "crdv7 (via GitHub)" <gi...@apache.org>.
crdv7 commented on code in PR #2517:
URL: https://github.com/apache/age/pull/2517#discussion_r3832130285
##########
src/backend/executor/cypher_delete.c:
##########
@@ -149,6 +149,9 @@ static TupleTableSlot *exec_cypher_delete(CustomScanState *node)
*/
while(true)
{
+ /* Release scratch retained by the preceding input row. */
+ ResetExprContext(econtext);
Review Comment:
Both the endpoint-index and sequential-scan connected-edge loops now reset their ExprContext before each RLS evaluation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Bound per-row memory in writable clauses [age]
Posted by "MuhammadTahaNaveed (via GitHub)" <gi...@apache.org>.
MuhammadTahaNaveed commented on code in PR #2517:
URL: https://github.com/apache/age/pull/2517#discussion_r3820973266
##########
src/backend/executor/cypher_merge.c:
##########
@@ -690,6 +717,9 @@ static TupleTableSlot *exec_cypher_merge(CustomScanState *node)
TupleTableSlot *projected;
HeapTuple htup;
+ /* Release scratch retained by the preceding input row. */
+ ResetExprContext(econtext);
Review Comment:
This resets the row context, but duplicate paths still appear to retain the pointer array allocated by `prebuild_path()`, since `free_path_entry_array()` only frees its entries. Could you check the cleanup in both duplicate branches?
##########
src/backend/executor/cypher_set.c:
##########
@@ -415,6 +418,13 @@ void apply_update_list(CustomScanState *node,
HASHCTL hashctl;
HTAB *index_cache = NULL;
HASHCTL idx_hashctl;
+ MemoryContext old_context;
+
+ /*
+ * All callers reset this context before processing the next input row.
+ * Keep transient update state here so it has the same bounded lifetime.
+ */
+ old_context = MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);
Review Comment:
I still see memory scaling with row count when the label has a stored generated column. Could you check whether all generated-column scratch is covered by this reset?
##########
src/backend/executor/cypher_delete.c:
##########
@@ -149,6 +149,9 @@ static TupleTableSlot *exec_cypher_delete(CustomScanState *node)
*/
while(true)
{
+ /* Release scratch retained by the preceding input row. */
+ ResetExprContext(econtext);
Review Comment:
Does this also cover the connected-edge RLS checks performed later during DELETE cleanup? Those checks appear to reuse the expression context without resetting it between edges.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Bound per-row memory in writable clauses [age]
Posted by "crdv7 (via GitHub)" <gi...@apache.org>.
crdv7 commented on PR #2517:
URL: https://github.com/apache/age/pull/2517#issuecomment-5372839971
> Some concerns:
>
> * SET memory appears to grow significantly when the label has a stored generated column. On the patchd build, the plain label case peaked around 56 MB RSS, while the generated column case reached roughly 743 MB.
> * Duplcate MERGE rows appear to retain the `prebuilt_path_array` container because only its entries are freed, causing memory usage to grow with the number of duplicate rows.
> * The DELETE expression context resets cover input rows, but the connected edge RLS checks apear to run without a reset between edges.
>
> Also, the attached benchmark runner does not implement the five run AB/BA process described in the PR, and its private memory sampling is fairly sparse for the shorter patched runs. This does not invalidate the performance improvement, but it makes the exact reported figures difficult to reproduce.
Thanks for the detailed review. I have force-pushed amended commit `560609e8` with the three fixes above.
The previous benchmark attachment was my mistake: I uploaded an internal AB/BA runner that required two manually prepared builds. I have replaced it with a self-contained runner that can be executed without arguments:
[reproduce_write_memory_benchmark.sh](https://github.com/user-attachments/files/31311914/reproduce_write_memory_benchmark.sh)
```bash
./reproduce_write_memory_benchmark.sh
```
With no source options, it downloads PostgreSQL 18.4 and AGE, then builds baseline AGE `801417404978823bd8732452c3f7959017584785` and patched AGE `560609e8092b0050dd026cb01e84fb2e6a1aff2a`. The runner starts isolated instances, runs five AB/BA-interleaved trials, samples private memory every 10 ms, and prints the final tables.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Bound per-row memory in writable clauses [age]
Posted by "crdv7 (via GitHub)" <gi...@apache.org>.
crdv7 commented on code in PR #2517:
URL: https://github.com/apache/age/pull/2517#discussion_r3832129231
##########
src/backend/executor/cypher_set.c:
##########
@@ -415,6 +418,13 @@ void apply_update_list(CustomScanState *node,
HASHCTL hashctl;
HTAB *index_cache = NULL;
HASHCTL idx_hashctl;
+ MemoryContext old_context;
+
+ /*
+ * All callers reset this context before processing the next input row.
+ * Keep transient update state here so it has the same bounded lifetime.
+ */
+ old_context = MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);
Review Comment:
Thanks, you were right. `ExecUpdateLockMode()` was initializing generated-column state in `es_query_cxt` for each temporary `ResultRelInfo`, retaining one copy per input row and causing memory to grow with the row count. The amended patch initializes that state in the EState per-tuple context for SET/REMOVE and DELETE.
##########
src/backend/executor/cypher_merge.c:
##########
@@ -690,6 +717,9 @@ static TupleTableSlot *exec_cypher_merge(CustomScanState *node)
TupleTableSlot *projected;
HeapTuple htup;
+ /* Release scratch retained by the preceding input row. */
+ ResetExprContext(econtext);
Review Comment:
`free_path_entry_array()` now frees both its entries and outer pointer array.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Bound per-row memory in writable clauses [age]
Posted by "MuhammadTahaNaveed (via GitHub)" <gi...@apache.org>.
MuhammadTahaNaveed merged PR #2517:
URL: https://github.com/apache/age/pull/2517
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org