Skip to content

[#1096] Copy a value into Persistit once rather than twice, and write the large values of PDBStorageTest through a small buffer pool - #1097

Open
vharseko wants to merge 1 commit into
OpenIdentityPlatform:masterfrom
vharseko:issues/1096-pdb-value-copy
Open

vharseko wants to merge 1 commit into
OpenIdentityPlatform:masterfrom
vharseko:issues/1096-pdb-value-copy

Conversation

@vharseko

@vharseko vharseko commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

Fixes #1096.

PDBStorageTest.testCanAddLargeValues still ran the 512 MB test JVM out of heap after #1072 - build-maven (ubuntu-latest, 17) of #1057, in toByteArray() of PDBStorage.bytesToValue() on the 63 MB put. This takes both remaining options named under "Left out" of #1072.

Why

On the way into Persistit the 63 MB value still had three humongous arrays live at once: the source, the toByteArray() copy, and the 64 MB value buffer of the exchange - on top of the buffer pool of the test storage (76 MB, 20% of the heap) and the test server. Under -Xmx512m a G1 region is 1 MB, so each 63 MB array needs a free run of 64 regions, which humongous objects already in place can break up: whether it is there is up to the collector's timing.

The change

Main - PDBStorage.bytesToValue(). The bytes are copied once, straight into the encoded bytes of the Value: the header is what Persistit itself writes for an empty byte array (putByteArray(new byte[0])), then ensureFit(len), copyTo(getEncodedBytes(), header), setEncodedSize(header + len). The bytes stored are the same as before - Persistit encodes a byte array as the header followed by the bytes as they are (checked in Value.putByteArray1 of the 3.1.2 in use) - and no private constant of Persistit is copied. Every put and update of a PDB backend saves one allocation of the size of its value.

Test - PDBStorageTest.

  • testCanAddLargeValues reopens its storage with a 16 MB buffer pool. The other methods keep db-cache-percent: the memory quota tests (aStorageWhoseOpenFailedGivesBackWhatItTook and its neighbours) are about that branch.
  • testValuesReadBackAsWritten (new) pins the encoding: an empty value, a ByteString past the offset of its array, a ByteStringBuilder, and a 64 KB patterned value which outgrows the encoded bytes the value had - so the bytes have to land in the array ensureFit() put in place of the old one.

Measured

testCanAddLargeValues alone, JDK 17.0.20, three runs per cell:

-Xmx master bytesToValue() only this PR
352m 3/3 pass 3/3 pass 3/3 pass
320m 3/3 OOM 3/3 pass 3/3 pass
288m 3/3 OOM 3/3 pass 3/3 pass
256m 3/3 OOM 3/3 OOM 3/3 pass
224m 3/3 OOM 3/3 OOM 3/3 OOM - on the allocation of the 63 MB source itself

Four mutants of bytesToValue() fail testValuesReadBackAsWritten: the bytes written over the header, no setEncodedSize(), the size without the header, and the encoded bytes read before ensureFit() (caught by the 64 KB value only: copyTo() truncates silently into the old array).

The whole class under -Xmx512m: 10/10 under JDK 17, 5/5 under JDK 26 on the first head (5031563).

Rebased onto master after #1066 (1aa253d), which touched the imports of PDBStorageTest as well: the only conflict was ResultCode and ByteStringBuilder added on the same line, and both are kept. The change itself is unchanged (git range-diff differs only in that context). On this head PDBStorageTest passes 28/28, the 13 cache size tests #1066 added included.

Left out

The -Xmx512m of the test fork in the root pom.xml stays as it is. bytesToKey() keeps its copy: keys are small, and Key.appendByteArray() escapes the bytes rather than taking them as they are.

@vharseko vharseko added bug tests Test suites: fixing, enabling, un-disabling java Changes to Java sources performance Performance / concurrency / lock-contention work labels Sep 24, 2026
…han twice, and write the large values of PDBStorageTest through a small buffer pool

bytesToValue() copied each value into a fresh array with toByteArray()
before Persistit copied it again into the value buffer of the exchange.
The bytes now go straight into the encoded bytes of the value, behind the
header Persistit itself writes for an empty byte array. For the 63 MB put
of PDBStorageTest that is one humongous array fewer live next to the
source and the 64 MB value buffer.

testCanAddLargeValues also reopens its storage with a 16 MB buffer pool
rather than 20% of the heap (76 MB under -Xmx512m); the other methods keep
the percentage, which the memory quota tests are about.

testCanAddLargeValues alone, JDK 17, three runs per heap: master runs out
of heap from 320m down, the bytesToValue() change alone from 256m down,
both together first at 224m, on the allocation of the 63 MB source itself.

testValuesReadBackAsWritten pins the encoding: an empty value, one past
the offset of its array, a ByteStringBuilder and a 64 KB value which
outgrows the encoded bytes read back as written.
@vharseko
vharseko force-pushed the issues/1096-pdb-value-copy branch from 5031563 to d0e7584 Compare September 24, 2026 15:44
@vharseko

Copy link
Copy Markdown
Member Author

@maximthomas I rebased onto master (d0e7584) after #1066 made this PR conflict. The only conflict was in the imports of PDBStorageTest: #1066 added ResultCode and this PR added ByteStringBuilder on the same line, and both are kept. PDBStorage.java merged on its own. The change itself is the same as before, and git range-diff differs only in that import context.

On this head PDBStorageTest passes 28/28. That includes testCanAddLargeValues, testValuesReadBackAsWritten and the 13 cache size tests from #1066. The description is updated to match. There were no review comments yet, so nothing else changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug java Changes to Java sources performance Performance / concurrency / lock-contention work tests Test suites: fixing, enabling, un-disabling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PDBStorageTest.testCanAddLargeValues still runs the 512 MB test JVM out of heap after #1072, on the toByteArray() copy of the 63 MB put

1 participant