You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "CritasWang (via GitHub)" <gi...@apache.org> on 2026/07/14 10:12:31 UTC
[PR] perf: single-pass tablet serialization — +52% write throughput, −61% latency [iotdb-client-nodejs]
CritasWang opened a new pull request, #16:
URL: https://github.com/apache/iotdb-client-nodejs/pull/16
## Problem
CPU profiling of an insertTablet-heavy workload (100 devices × 20 sensors × 1000-row batches, 20 clients) showed the event loop 79.5% busy, with `serializeTabletValues` at 26% self time and GC at 24.5% — the write path was dominated by allocation churn, not I/O:
1. `serializeTabletValues` transposed rows→columns (`values.map(row => row[col])` — a fresh array per column), built a `boolean[]` null bitmap per column even when the column had no nulls, serialized each column into its own intermediate buffer, and finally `Buffer.concat`-ed everything (a full payload re-copy).
2. INT64/TIMESTAMP writes allocated a `BigInt` per value for `writeBigInt64BE`.
3. TEXT/STRING columns re-encoded UTF-8 for every row, even constant TAG columns.
4. `BufferPool` was acquire-only (zero `release()` calls in `src/`) — 0% hit rate, pure overhead over `Buffer.allocUnsafe`.
5. `serializeBitMaps` allocated a 1-byte `Buffer.from([flag])` per column.
## Changes
- **Single-pass, single-buffer tablet serialization** (`serializeTabletValuesFast`): pre-compute the exact payload size (fixed widths from schema; one `byteLength` scan for variable-width columns), allocate one buffer, write values column-by-column reading `values[row][col]` directly, and pack null bitmaps inline — flag byte stays 0 and no bitmap bytes are emitted when a column has no nulls. No transpose, no intermediate buffers, no `Buffer.concat`.
- **BigInt-free int64 writes**: sign-correct hi/lo 32-bit pair (`hi = Math.floor(v / 2^32)`, `lo = v >>> 0`); BigInt inputs and non-safe integers still use `writeBigInt64BE`.
- **String encode cache**: single-entry memo reuses the encoded buffer for consecutive identical strings (constant TAG columns).
- **BufferPool deprecated**: removed from the write path (kept exported for API compatibility, marked `@deprecated`).
The legacy path (`enableFastSerialization: false`) is unchanged.
## Results (IoTDB 2.0.10 standalone, 16-core box, DOUBLE workload, 10^9 points/run, interleaved A/B, 2 rounds)
| Run | Throughput | Avg latency | P99 |
|---|---|---|---|
| baseline #1 | 8.70M pts/s | 26.83 ms | 78.11 ms |
| optimized #1 | 13.11M pts/s | 10.60 ms | 23.61 ms |
| baseline #2 | 8.53M pts/s | 27.07 ms | 78.90 ms |
| optimized #2 | 13.16M pts/s | 10.53 ms | 23.18 ms |
**+52% throughput, −61% average latency, −70% P99.** Server-side `count(*)` verified 100,000,000 rows for every run. Serializer self time dropped from 26% to 14.2% in `--cpu-prof`; the profiled process now shows 16% idle where it was previously saturated.
## Wire compatibility
- All pre-existing serialization unit tests (which assert exact wire bytes) pass unchanged.
- New golden tests assert the fast path equals the legacy path **byte-for-byte** for mixed-type tablets with nulls, all-null columns, bitmap-width edge cases, BLOB/DATE, and negative/`MIN_SAFE_INTEGER`/BigInt int64 values.
- New unit tests cover hi/lo int64 correctness (two's complement negatives, i64 min/max BigInt) and string-cache correctness.
- e2e `AllDataTypes` + `TableModelDataTypes` suites pass against a live IoTDB 2.0.10.
## Caveats
- `BufferPool` is deprecated but still exported; removal would be semver-major.
- The remaining GC in profiles is dominated by the benchmark harness's own tablet construction, not the client.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] perf: single-pass tablet serialization — +52% write throughput, −61% latency [iotdb-client-nodejs]
Posted by "CritasWang (via GitHub)" <gi...@apache.org>.
CritasWang commented on code in PR #16:
URL: https://github.com/apache/iotdb-client-nodejs/pull/16#discussion_r3584475804
##########
.specify/memory/constitution.md:
##########
@@ -0,0 +1,50 @@
+# [PROJECT_NAME] Constitution
Review Comment:
Removed — all `.claude/` and `.specify/` files are dropped from the PR (they were accidental local scaffolding; the diff is now 7 files / +611). Sorry for the noise. (c5f5522)
##########
src/utils/FastSerializer.ts:
##########
@@ -265,3 +278,245 @@ export function serializeColumnFast(values: any[], dataType: number): Buffer {
throw new Error(`Unsupported data type: ${dataType}`);
}
}
+
+/**
+ * Serialize a whole tablet's values (all columns + null bitmaps) into ONE buffer.
+ *
+ * Wire format (identical to the legacy per-column path):
+ * [col0 data][col1 data]...[colN data]
+ * then per column: [hasNull flag byte][bitmap bytes, only when flag=1]
+ * Bitmap: ceil(rowCount/8) bytes, LSB-first, bit=1 means NULL.
+ *
+ * Optimizations over the legacy path:
+ * - No rows→columns transpose (reads values[row][col] directly)
+ * - No per-column intermediate buffers or boolean[] bitmaps
+ * - No trailing Buffer.concat (exact data size pre-computed; bitmap section
+ * allocated worst-case and trimmed with a zero-copy subarray)
+ * - Null bitmaps are packed inline during the value pass; the flag byte
+ * stays 0 and no bitmap bytes are emitted when a column has no nulls
+ *
+ * @param values Row-major tablet values: values[rowIndex][colIndex]
+ * @param dataTypes TSDataType code per column
+ * @param rowCount Number of rows
+ */
+export function serializeTabletValuesFast(
+ values: any[][],
+ dataTypes: number[],
+ rowCount: number,
+): Buffer {
+ const numCols = dataTypes.length;
+ const bitmapBytes = Math.ceil(rowCount / 8);
+
+ // ---- Pass 1: exact data-section size (fixed widths from schema; one
+ // byteLength scan for variable-width columns, memoizing repeated strings) ----
+ let dataSize = 0;
+ for (let c = 0; c < numCols; c++) {
+ switch (dataTypes[c]) {
+ case 0: // BOOLEAN
+ dataSize += rowCount;
+ break;
+ case 1: // INT32
+ case 3: // FLOAT
+ case 9: // DATE
+ dataSize += rowCount * 4;
+ break;
+ case 2: // INT64
+ case 4: // DOUBLE
+ case 8: // TIMESTAMP
+ dataSize += rowCount * 8;
+ break;
+ case 5: // TEXT
+ case 11: { // STRING
+ let lastStr: string | null = null;
+ let lastLen = 0;
+ for (let r = 0; r < rowCount; r++) {
+ const v = values[r][c];
+ if (v === null || v === undefined) {
+ dataSize += 4;
+ continue;
+ }
+ const str = typeof v === "string" ? v : String(v);
+ if (str !== lastStr) {
+ lastLen = Buffer.byteLength(str, "utf8");
+ lastStr = str;
+ }
+ dataSize += 4 + lastLen;
+ }
+ break;
+ }
+ case 10: { // BLOB
+ for (let r = 0; r < rowCount; r++) {
+ const v = values[r][c];
+ dataSize += 4 + (v === null || v === undefined
+ ? 0
+ : Buffer.isBuffer(v) ? v.length : Buffer.from(v).length);
Review Comment:
Fixed in c5f5522: the sizing pass now uses a new `blobByteLength()` helper (no allocation — `Uint8Array`/`Buffer` use `.length`, strings use `Buffer.byteLength`, array-likes use `.length`), and the write pass writes directly into the target buffer (`buffer.set` for `Uint8Array`, `buffer.write` for strings, byte loop for array-likes) with no intermediate `Buffer.from`. Re-ran your microbenchmark scenario (1000 × 64-byte `Uint8Array` cells): 0.127 ms/tablet before → 0.051 ms/tablet after (~2.5× faster, and faster than the pre-PR baseline). Added a golden fast-vs-legacy test covering `Uint8Array`, byte-array, string, empty, and null BLOB inputs.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] perf: single-pass tablet serialization — +52% write throughput, −61% latency [iotdb-client-nodejs]
Posted by "CritasWang (via GitHub)" <gi...@apache.org>.
CritasWang commented on PR #16:
URL: https://github.com/apache/iotdb-client-nodejs/pull/16#issuecomment-4976159906
Rebased onto develop after #15 (DATE yyyyMMdd fix) merged. Conflict resolution notes: kept #15's `parseDateToInt` semantics everywhere and removed the BufferPool usage from `serializeDateColumn`; also updated the fast tablet path's DATE branch to use `parseDateToInt` (it predated #15), so the single-buffer path encodes yyyyMMdd identically — covered by the existing golden fast-vs-legacy byte-for-byte tests plus the exact-wire-bytes DATE tests from #15. 152/152 unit tests pass.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] perf: single-pass tablet serialization — +52% write throughput, −61% latency [iotdb-client-nodejs]
Posted by "HTHou (via GitHub)" <gi...@apache.org>.
HTHou merged PR #16:
URL: https://github.com/apache/iotdb-client-nodejs/pull/16
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] perf: single-pass tablet serialization — +52% write throughput, −61% latency [iotdb-client-nodejs]
Posted by "CritasWang (via GitHub)" <gi...@apache.org>.
CritasWang commented on PR #16:
URL: https://github.com/apache/iotdb-client-nodejs/pull/16#issuecomment-4976895260
Also addressed the `Config.enableFastSerialization` JSDoc (outside the original diff, included in c5f5522): it no longer mentions buffer pooling or the 2-3x claim — it now describes the actual implementation (single-pass single-buffer writes, inline null bitmaps, BigInt-free int64) and notes the output is byte-identical to the legacy path. All 153 unit tests pass; lint at baseline (61 errors, none new).
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] perf: single-pass tablet serialization — +52% write throughput, −61% latency [iotdb-client-nodejs]
Posted by "HTHou (via GitHub)" <gi...@apache.org>.
HTHou commented on code in PR #16:
URL: https://github.com/apache/iotdb-client-nodejs/pull/16#discussion_r3584365879
##########
src/utils/FastSerializer.ts:
##########
@@ -265,3 +278,245 @@ export function serializeColumnFast(values: any[], dataType: number): Buffer {
throw new Error(`Unsupported data type: ${dataType}`);
}
}
+
+/**
+ * Serialize a whole tablet's values (all columns + null bitmaps) into ONE buffer.
+ *
+ * Wire format (identical to the legacy per-column path):
+ * [col0 data][col1 data]...[colN data]
+ * then per column: [hasNull flag byte][bitmap bytes, only when flag=1]
+ * Bitmap: ceil(rowCount/8) bytes, LSB-first, bit=1 means NULL.
+ *
+ * Optimizations over the legacy path:
+ * - No rows→columns transpose (reads values[row][col] directly)
+ * - No per-column intermediate buffers or boolean[] bitmaps
+ * - No trailing Buffer.concat (exact data size pre-computed; bitmap section
+ * allocated worst-case and trimmed with a zero-copy subarray)
+ * - Null bitmaps are packed inline during the value pass; the flag byte
+ * stays 0 and no bitmap bytes are emitted when a column has no nulls
+ *
+ * @param values Row-major tablet values: values[rowIndex][colIndex]
+ * @param dataTypes TSDataType code per column
+ * @param rowCount Number of rows
+ */
+export function serializeTabletValuesFast(
+ values: any[][],
+ dataTypes: number[],
+ rowCount: number,
+): Buffer {
+ const numCols = dataTypes.length;
+ const bitmapBytes = Math.ceil(rowCount / 8);
+
+ // ---- Pass 1: exact data-section size (fixed widths from schema; one
+ // byteLength scan for variable-width columns, memoizing repeated strings) ----
+ let dataSize = 0;
+ for (let c = 0; c < numCols; c++) {
+ switch (dataTypes[c]) {
+ case 0: // BOOLEAN
+ dataSize += rowCount;
+ break;
+ case 1: // INT32
+ case 3: // FLOAT
+ case 9: // DATE
+ dataSize += rowCount * 4;
+ break;
+ case 2: // INT64
+ case 4: // DOUBLE
+ case 8: // TIMESTAMP
+ dataSize += rowCount * 8;
+ break;
+ case 5: // TEXT
+ case 11: { // STRING
+ let lastStr: string | null = null;
+ let lastLen = 0;
+ for (let r = 0; r < rowCount; r++) {
+ const v = values[r][c];
+ if (v === null || v === undefined) {
+ dataSize += 4;
+ continue;
+ }
+ const str = typeof v === "string" ? v : String(v);
+ if (str !== lastStr) {
+ lastLen = Buffer.byteLength(str, "utf8");
+ lastStr = str;
+ }
+ dataSize += 4 + lastLen;
+ }
+ break;
+ }
+ case 10: { // BLOB
+ for (let r = 0; r < rowCount; r++) {
+ const v = values[r][c];
+ dataSize += 4 + (v === null || v === undefined
+ ? 0
+ : Buffer.isBuffer(v) ? v.length : Buffer.from(v).length);
Review Comment:
[P2] Avoid materializing non-`Buffer` BLOB values twice.
The sizing pass calls `Buffer.from(v)` here, and the write pass calls it again for the same value. The previous serializer normalized each value once and reused that buffer, so existing accepted inputs such as `Uint8Array` and number arrays now allocate two temporary buffers per cell. In a local 1,000 × 64-byte `Uint8Array` microbenchmark, the new tablet path was roughly 30–40% slower. Please determine the length without allocating where possible, or retain the normalized buffer for the write pass, and add coverage for non-`Buffer` BLOB inputs.
##########
.specify/memory/constitution.md:
##########
@@ -0,0 +1,50 @@
+# [PROJECT_NAME] Constitution
Review Comment:
[P1] Please remove the unrelated SpecKit/Claude scaffolding from this PR.
This change adds 20 files under `.claude/` and `.specify/`—most of the PR’s 4,026 added lines—including placeholder governance content and executable scripts, but none of them are related to tablet serialization. Merging them would unexpectedly expand the repository’s tooling and governance surface, and these new files also lack ASF license headers. Please drop all `.claude/` and `.specify/` additions here; if they are wanted, they should be proposed separately with their own rationale and review.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] perf: single-pass tablet serialization — +52% write throughput, −61% latency [iotdb-client-nodejs]
Posted by "CritasWang (via GitHub)" <gi...@apache.org>.
CritasWang commented on PR #16:
URL: https://github.com/apache/iotdb-client-nodejs/pull/16#issuecomment-4977705512
Addressed the ArrayBuffer P2 in 5f4c1e0: `blobByteLength()` now handles `ArrayBuffer`/`SharedArrayBuffer` via `.byteLength` (they have no `.length`, which previously made `dataSize` NaN → `RangeError`), and the write pass bulk-copies through a zero-copy `new Uint8Array(v)` view via `buffer.set()`. Added a golden fast-vs-legacy test covering `ArrayBuffer`, `SharedArrayBuffer`, empty `ArrayBuffer`, and null. Reproduced the failure first (RangeError, exactly as you described), then verified the fix end-to-end. 154/154 unit tests pass; lint at baseline (61, none new).
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org