Skip to content

Commit

Permalink
Merge pull request #1119 from finos/fix-postmessage
Browse files Browse the repository at this point in the history
Fix transferable list in worker.postMessage
  • Loading branch information
texodus authored Jul 26, 2020
2 parents 9327333 + 1fcd59d commit 974807e
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 4 deletions.
Binary file not shown.
59 changes: 59 additions & 0 deletions packages/perspective-viewer/test/js/regressions.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@

const utils = require("@finos/perspective-test");
const path = require("path");
const arrows = require("./test_arrows.js");

const arraybuffer_to_string = function(buf) {
return String.fromCharCode(...new Uint8Array(buf));
};

utils.with_server({}, () => {
describe.page(
Expand Down Expand Up @@ -56,6 +61,60 @@ utils.with_server({}, () => {
},
{wait_for_update: false}
);

test.capture(
"When transferables are enabled, transfers an arrow in load.",
async page => {
const viewer = await page.$("perspective-viewer");
const arrow = arrows.int_float_str_arrow.slice();
const byte_length = await page.evaluate(
async (viewer, data) => {
const arrow = Uint8Array.from([...data].map(ch => ch.charCodeAt())).buffer;
viewer.load(arrow);
// force _process to run - otherwise reading
// bytelength will return the un-transfered arrow.
await viewer.table.size();
return arrow.byteLength;
},
viewer,
arraybuffer_to_string(arrow)
);
await page.waitForSelector("perspective-viewer:not([updating])");
expect(byte_length).toBe(0);
},
{wait_for_update: false}
);

test.capture(
"When transferables are enabled, transfers an arrow in update.",
async page => {
const viewer = await page.$("perspective-viewer");
const schema = {
a: "integer",
b: "float",
c: "string"
};
const arrow = arrows.int_float_str_arrow.slice();
console.log(arrow.byteLength);
const byte_length = await page.evaluate(
async (viewer, data, schema) => {
viewer.load(schema);
const arrow = Uint8Array.from([...data].map(ch => ch.charCodeAt())).buffer;
viewer.update(arrow);
// force _process to run - otherwise reading
// bytelength will return the un-transfered arrow.
await viewer.table.size();
return arrow.byteLength;
},
viewer,
arraybuffer_to_string(arrow),
schema
);
await page.waitForSelector("perspective-viewer:not([updating])");
expect(byte_length).toBe(0);
},
{wait_for_update: false}
);
},
{root: path.join(__dirname, "..", "..")}
);
Expand Down
34 changes: 34 additions & 0 deletions packages/perspective-viewer/test/js/test_arrows.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/******************************************************************************
*
* Copyright (c) 2019, the Perspective Authors.
*
* This file is part of the Perspective library, distributed under the terms of
* the Apache License 2.0. The full license can be found in the LICENSE file.
*
*/
const fs = require("fs");
const path = require("path");

/**
* Returns an `ArrayBuffer` containing the contents of a `.arrow` file
* located at `arrow_path`.
*
* Because `fs.readFileSync` shares its underlying buffer
* between calls to `readFileSync`, we need to get a slice
* of the `ArrayBuffer` specifically at its byte offset.
*
* See https://github.com/nodejs/node/issues/11132 for more details.
*
* @param arrow_path {String} a path to an arrow file.
* @returns {ArrayBuffer} an ArrayBuffer containing the arrow-serialized data.
*/
function load_arrow(arrow_path) {
const data = fs.readFileSync(arrow_path);
return data.buffer.slice(data.byteOffset, data.byteOffset + data.byteLength);
}

const int_float_str_arrow = load_arrow(path.join(__dirname, "..", "arrow", "int_float_str.arrow"));

module.exports = {
int_float_str_arrow
};
6 changes: 4 additions & 2 deletions packages/perspective-viewer/test/results/linux.docker.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
"Computed_Expressions_On_restore,_computed_expressions_in_classic_syntax_are_parsed_correctly_": "563f8c45e0ec32bc4b07d30ed5f0086e",
"superstore_adds_computed_column_via_attribute": "f2fa526d6f0c168a47ab272bb7ae5771",
"superstore_user_defined_aggregates_maintained_on_computed_columns": "c7aa2902feb512ceb1e1487de80878ff",
"__GIT_COMMIT__": "410648f3426fe2358cdf2a9a89424ed6cbe6122a",
"__GIT_COMMIT__": "2df957aa0feae17b14622bcd2cc489b5421215fd",
"blank_Handles_reloading_with_a_schema_": "9d171ff4c95a7f31eff2e7127b8caedf",
"superstore_doesn_t_leak_tables_": "5bb762b2860eb5af067bb83afbca3264",
"superstore_doesn_t_leak_elements_": "5bb762b2860eb5af067bb83afbca3264",
Expand All @@ -75,5 +75,7 @@
"superstore_displays_visible_columns_": "7a7118d6cdb7ce751d5fbb5a36141e6a",
"superstore_Responsive_Layout_shows_horizontal_columns_on_small_vertical_viewports_": "d90b1544ef1efa405b6eb68ee136e4af",
"superstore_Responsive_Layout_shows_horizontal_columns_on_small_vertical_and_horizontal_viewports_": "e51afee0ec45554b3a27d1ec313305f6",
"superstore_replaces_all_rows_": "03995f23192c53d36160ab55127ff5c0"
"superstore_replaces_all_rows_": "03995f23192c53d36160ab55127ff5c0",
"blank_When_transferables_are_enabled,_transfers_an_arrow_in_load_": "dcfab9d5536933ba80b87b503985f398",
"blank_When_transferables_are_enabled,_transfers_an_arrow_in_update_": "dcfab9d5536933ba80b87b503985f398"
}
2 changes: 1 addition & 1 deletion packages/perspective/src/js/api/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ export class Server {
if (msg.args && msg.args[0]) {
if (msg.method === "on_update" && msg.args[0]["mode"] === "row") {
// actual arrow is in the `delta`
this.post(result, [ev.delta]);
this.post(result, ev.delta);
return;
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/perspective/src/js/perspective.parallel.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ class WebWorkerClient extends Client {
*/
send(msg) {
if (this._worker.transferable && msg.args && msg.args[0] instanceof ArrayBuffer) {
this._worker.postMessage(msg, msg.args[0]);
this._worker.postMessage(msg, [msg.args[0]]);
} else {
this._worker.postMessage(msg);
}
Expand Down

0 comments on commit 974807e

Please sign in to comment.