This is an automated email from the ASF dual-hosted git repository.
chaokunyang pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/fory.git
The following commit(s) were added to refs/heads/main by this push:
new e823e471c fix(javascript): reserve writer capacity for write paths
(#3994)
e823e471c is described below
commit e823e471ccfb3a7fdcd0da62c5b3b2adc96a2c17
Author: Ayush Kumar <[email protected]>
AuthorDate: Sun Aug 30 22:35:41 2026 +0530
fix(javascript): reserve writer capacity for write paths (#3994)
## Why?
BinaryWriter starts with a ~100KB buffer and, for speed, its individual
writeXxx methods do no bounds checking. The contract is that callers
call reserve(n) first, which grows the buffer if needed. The generated
collection write path honors this (it reserves elementFixedSize * count
before the loop), but three paths write per-entry data with no reserve
at all: the generated declared-map write, MapAnySerializer.write, and
CollectionAnySerializer.write.
## What does this PR do?
three write paths now reserve writer capacity, matching the existing
generated-collection convention (root reserves fixedSize once;
per-element loops reserve fixedSize × count; strings reserve internally)
## Related issues
## AI Contribution Checklist
- [ ] Substantial AI assistance was used in this PR: `yes` / `no`
- [ ] If `yes`, I included a completed [AI Contribution
Checklist](https://github.com/apache/fory/blob/main/AI_POLICY.md#9-contributor-checklist-for-ai-assisted-prs)
in this PR description and the required `AI Usage Disclosure`.
- [ ] If `yes`, my PR description includes the required `ai_review`
summary and screenshot evidence or equivalent persisted links of the
final clean AI review results from both fresh reviewers described in
`AI_POLICY.md`, the Fory-guided reviewer and the independent general
reviewer, on the current PR diff or current HEAD after the latest code
changes.
## Does this PR introduce any user-facing change?
- [ ] Does this PR introduce any public API change?
- [ ] Does this PR introduce any binary protocol compatibility change?
## Benchmark
---
javascript/packages/core/lib/gen/collection.ts | 8 +++++
javascript/packages/core/lib/gen/map.ts | 8 +++++
javascript/test/array.test.ts | 41 ++++++++++++++++++++++++++
javascript/test/map.test.ts | 32 ++++++++++++++++++++
4 files changed, 89 insertions(+)
diff --git a/javascript/packages/core/lib/gen/collection.ts
b/javascript/packages/core/lib/gen/collection.ts
index 8ac390074..e4ae50431 100644
--- a/javascript/packages/core/lib/gen/collection.ts
+++ b/javascript/packages/core/lib/gen/collection.ts
@@ -241,6 +241,7 @@ export class CollectionAnySerializer {
this.writeElementsHeader(value);
if (isSame) {
serializer!.writeTypeInfo(sample);
+ this.writeContext.writer.reserve((serializer!.fixedSize + 1) * size);
if (trackingRef) {
for (const item of value) {
if (!serializer!.writeRefOrNull(item)) {
@@ -262,12 +263,16 @@ export class CollectionAnySerializer {
}
}
} else {
+ // Mixed-type elements resolve a serializer per item, so capacity is
+ // reserved per item; the upfront byte per element covers null flags.
+ this.writeContext.writer.reserve(size);
if (trackingRef) {
for (const item of value) {
if (item === null || item === undefined) {
this.writeContext.writer.writeInt8(RefFlags.NullFlag);
} else {
const serializer =
this.writeContext.typeResolver.getSerializerByData(item);
+ this.writeContext.writer.reserve(serializer!.fixedSize);
serializer!.writeRef(item);
}
}
@@ -277,6 +282,7 @@ export class CollectionAnySerializer {
this.writeContext.writer.writeInt8(RefFlags.NullFlag);
} else {
const serializer =
this.writeContext.typeResolver.getSerializerByData(item);
+ this.writeContext.writer.reserve(serializer!.fixedSize);
this.writeContext.writer.writeInt8(RefFlags.NotNullValueFlag);
serializer!.writeNoRef(item);
}
@@ -284,6 +290,7 @@ export class CollectionAnySerializer {
} else {
for (const item of value) {
const serializer =
this.writeContext.typeResolver.getSerializerByData(item);
+ this.writeContext.writer.reserve(serializer!.fixedSize);
serializer!.writeNoRef(item);
}
}
@@ -307,6 +314,7 @@ export class CollectionAnySerializer {
}
}
this.writeContext.writer.writeUint8(flags);
+ this.writeContext.writer.reserve((serializer.fixedSize + 1) * size);
if (flags & CollectionFlags.TRACKING_REF) {
for (const item of value) {
if (!serializer.writeRefOrNull(item)) {
diff --git a/javascript/packages/core/lib/gen/map.ts
b/javascript/packages/core/lib/gen/map.ts
index 3e1882354..d9fcdebf3 100644
--- a/javascript/packages/core/lib/gen/map.ts
+++ b/javascript/packages/core/lib/gen/map.ts
@@ -209,6 +209,11 @@ export class MapAnySerializer {
this.valueSerializer !== null
? this.valueSerializer
: this.writeContext.typeResolver.getSerializerByData(v);
+ this.writeContext.writer.reserve(
+ (keySerializer ? keySerializer.fixedSize : 1) +
+ (valueSerializer ? valueSerializer.fixedSize : 1) +
+ 2,
+ );
const header = mapChunkWriter.next(
new ElementInfo(
@@ -421,6 +426,9 @@ export class MapSerializerGenerator extends
BaseSerializerGenerator {
return `
${this.builder.writer.writeVarUint32Small7(`${accessor}.size`)}
+ ${this.builder.writer.reserve(
+ `${this.keyGenerator.getFixedSize() +
this.valueGenerator.getFixedSize() + 2} * ${accessor}.size`,
+ )};
let ${lastKeyIsNull} = false;
let ${lastValueIsNull} = false;
let ${chunkSize} = 0;
diff --git a/javascript/test/array.test.ts b/javascript/test/array.test.ts
index 5637d0f78..d6b66a4b0 100644
--- a/javascript/test/array.test.ts
+++ b/javascript/test/array.test.ts
@@ -323,6 +323,47 @@ describe("array", () => {
);
expect(containsBytes(bfloat16Bytes, [0x80, 0x3f, 0x00, 0xc0])).toBe(true);
});
+
+ test("should large any-typed list work", () => {
+ // The dynamic element write path must reserve writer capacity per item.
+ // Without it, single-byte writes past the buffer end were silent no-ops
+ // while the cursor advanced, so dump() returned uninitialized tail bytes.
+ const fory = new Fory({ compatible: false });
+ const { serialize, deserialize } = fory.register(Type.list(Type.any()));
+ const arr = new Array(150000).fill(1);
+ const result = deserialize(serialize(arr)) as number[];
+ expect(result.length).toBe(150000);
+ expect(result.every((x) => x === 1)).toBe(true);
+ });
+
+ test("should large mixed-type list work", () => {
+ // Mixed element types disable the same-type aggregate reserve, so this
+ // exercises the per-item reserves in the dynamic write loops, with and
+ // without null elements. Numeric elements only: string bodies reserve
+ // internally, which would mask a missing per-item reserve.
+ const fory = new Fory({ compatible: false });
+ const { serialize, deserialize } = fory.register(Type.list(Type.any()));
+ const arr: (number | bigint | null)[] = [];
+ for (let i = 0; i < 50000; i++) {
+ arr.push(i, BigInt(i), i % 100 === 0 ? null : -i);
+ }
+ expect(deserialize(serialize(arr))).toEqual(arr);
+
+ const noNulls = arr.filter((x) => x !== null);
+ expect(deserialize(serialize(noNulls))).toEqual(noNulls);
+ });
+
+ test("should reserialize unknown struct with a large declared list", () => {
+ // Reserializing an unknown compatible struct writes declared list fields
+ // through CollectionAnySerializer.writeDeclared, which must reserve
+ // writer capacity for the whole list body.
+ const writerFory = new Fory({ compatible: true });
+ const readerFory = new Fory({ compatible: true });
+ const writer = writerFory.register(Type.struct(7501, { values:
Type.list(Type.int32()) }));
+ const values = new Array(30000).fill(123456789);
+ const unknown = readerFory.deserialize(writer.serialize({ values }));
+ expect(writer.deserialize(readerFory.serialize(unknown))).toEqual({ values
});
+ });
});
function containsBytes(bytes: Uint8Array, needle: number[]) {
diff --git a/javascript/test/map.test.ts b/javascript/test/map.test.ts
index 55f8cbf86..367170aff 100644
--- a/javascript/test/map.test.ts
+++ b/javascript/test/map.test.ts
@@ -213,4 +213,36 @@ describe("map", () => {
expect(serializer.deserialize(valid)).toEqual(value);
}
});
+
+ test("should large declared map work", () => {
+ // The generated map write must reserve writer capacity for its entries.
+ // Without it, unchecked DataView writes past the buffer end threw a
+ // RangeError once the map body outgrew the initial buffer.
+ const fory = new Fory({ compatible: false });
+ const { serialize, deserialize } = fory.register(
+ Type.struct(
+ { namespace: "example", typeName: "BigMap" },
+ { m: Type.map(Type.int32({ encoding: "fixed" }), Type.int32({
encoding: "fixed" })) },
+ ),
+ );
+ const m = new Map<number, number>();
+ for (let i = 0; i < 30000; i++) {
+ m.set(i, i + 1);
+ }
+ expect(deserialize(serialize({ m })).m.get(29999)).toBe(30000);
+ });
+
+ test("should large any-typed map work", () => {
+ // A map with dynamic key/value types writes through MapAnySerializer,
+ // which must reserve writer capacity per entry.
+ // Numeric entries only: string bodies reserve internally, which would
+ // mask a missing per-entry reserve.
+ const fory = new Fory({ compatible: false });
+ const { serialize, deserialize } = fory.register(Type.map(Type.any(),
Type.any()));
+ const m = new Map<any, any>();
+ for (let i = 0; i < 30000; i++) {
+ m.set(i, i % 2 === 0 ? BigInt(i) : i * 3);
+ }
+ expect(deserialize(serialize(m))).toEqual(m);
+ });
});
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
