trxcllnt commented on code in PR #14:
URL: https://github.com/apache/arrow-js/pull/14#discussion_r2112535602
##########
src/ipc/writer.ts:
##########
@@ -251,8 +269,12 @@ export class RecordBatchWriter<T extends TypeMap = any>
extends ReadableInterop<
}
protected _writeRecordBatch(batch: RecordBatch<T>) {
+ if (this._compression != null) {
+ return this._writeCompressedRecordBatch(batch);
+ }
+
const { byteLength, nodes, bufferRegions, buffers } =
VectorAssembler.assemble(batch);
- const recordBatch = new metadata.RecordBatch(batch.numRows, nodes,
bufferRegions);
+ const recordBatch = new metadata.RecordBatch(batch.numRows, nodes,
bufferRegions, null);
Review Comment:
IIUC this could be simplified a bit. Instead of having a separate
`_writeCompressedRecordBatch()` method, what about adding a method to assemble
and optionally compress? e.g.
```typescript
protected _writeRecordBatch(batch: RecordBatch<T>) {
const { byteLength, nodes, bufferRegions, buffers } =
this._assembleRecordBatch(batch);
const recordBatch = new metadata.RecordBatch(batch.numRows, nodes,
bufferRegions, this._compression);
const message = Message.from(recordBatch, byteLength);
return this
._writeDictionaries(batch)
._writeMessage(message)
._writeBodyBuffers(buffers);
}
protected _assembleRecordBatch(batch: RecordBatch<T>) {
let { byteLength, nodes, bufferRegions, buffers } =
VectorAssembler.assemble(batch);
if (this._compression != null) {
({byteLength, bufferRegions, buffers } =
this._compressBodyBuffers(buffers));
}
return { byteLength, nodes, bufferRegions, buffers };
}
protected _writeBodyBuffers(buffers: ArrayBufferView[]) {
const stride = this._compression != null ? 2 : 1;
const bufs = new Array(stride);
for (let i = 0, n = buffers.length; i < n; i += stride) {
for (let j = -1; ++j < stride;) {
bufs[j] = buffers[i + j];
}
const size = bufs.reduce((size, buf) => size + buf.byteLength, 0);
if (size > 0) {
bufs.forEach((buf) => this._write(buf));
const padding = ((size + 7) & ~7) - size;
if (padding > 0) {
this._writePadding(padding);
}
}
}
return this;
}
```
In the above, `_compressBodyBuffers` would contain the loop that
`_writeCompressedRecordBatch` does, but its `buffers` field is a flat list of
ArrayBufferViews (instead of `[Uint8Array, Uint8Array]` tuples) so it matches
the existing structure.
How does that sound?
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
