Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transfer buffers to/from worker to avoid cloning #113

Merged
merged 1 commit into from
May 4, 2020
Merged

Transfer buffers to/from worker to avoid cloning #113

merged 1 commit into from
May 4, 2020

Conversation

axelboc
Copy link
Contributor

@axelboc axelboc commented May 4, 2020

This should improve the performance of texture computations.

@axelboc
Copy link
Contributor Author

axelboc commented May 4, 2020

From a completely unscientific comparison of the loading animations in prod and in the deploy preview, it looks like it takes about a third of the time to compute the texture with the optimisation than it does without!

@axelboc
Copy link
Contributor Author

axelboc commented May 4, 2020

Something strange, though, which also occured without the optimisation:

  • when you open a dataset and change the domain, every change takes about 1/3 loading animation;
  • but when you change to another color map, every subsequent change of domain takes about 4/3 loading animations (one full horizontal "scan" + 1/3)

@axelboc
Copy link
Contributor Author

axelboc commented May 4, 2020

Never mind, some color scales just take a lot more computation time than others. 😅

mergeState({ loading: true });

// Prepare transferable buffer to avoid cloning values array
const typedValues = Float64Array.from(values);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Float 64 is the type for numbers in JS. Eventually, we may have to parse the JSON (or whatever format) returned by the server and create the appropriate typed array right away to optimise things further.

mergeState({
loading: false,
textureData: await proxy.computeTextureData(
values,
transfer(typedValues, [typedValues.buffer]),
Copy link
Contributor Author

@axelboc axelboc May 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -16,12 +16,15 @@ function computeTextureData(
const colorScale = scaleSequential(INTERPOLATORS[colorMap]);

// Compute RGB color array for each datapoint `[[<r>, <g>, <b>], [<r>, <g>, <b>], ...]`
const colors = values.map(val => {
const colors = values.reduce<number[]>((acc, val) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I replace map with reduce to avoid having to flatten the array afterwards.

@axelboc axelboc merged commit 19cb9bc into master May 4, 2020
@axelboc axelboc deleted the optim branch May 4, 2020 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants