Skip to content

Jp/first workers#629

Merged
TheJeran merged 11 commits intomainfrom
jp/first-workers
Apr 15, 2026
Merged

Jp/first workers#629
TheJeran merged 11 commits intomainfrom
jp/first-workers

Conversation

@TheJeran
Copy link
Copy Markdown
Collaborator

This is the first batch of workers for array tasks as mentioned in #601. It just applies to GetCurrentArray() with its counterpart GetCurrentArrayWorkers(). For uncompressed data there isn't a very notable speed-up. Maybe on very large datasets. But for compressed data the speed-up is very noticeable.

Trying to think of the most efficient application of workers to the data fetching itself. At the Moment may just create workers for data conversion as I haven't found a way to read netcdf data within the workers. Zarr can fetch in workers, but each worker must initialize the store every time which is inefficient.

Also fixed a bug in the new GetArray() function which looks at fetchNC value. However, this can be true even wheil fetching zarr. So now it checks if fetchNC is true AND the initstore is Local. Then it's a NC for sure.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces parallelized data processing using Web Workers and SharedArrayBuffer through a new GetCurrentArrayWorkers utility. Corresponding updates were made to AnalysisWG and FlatMap to support asynchronous data retrieval. Feedback highlights critical compatibility concerns regarding SharedArrayBuffer and cross-origin isolation, a potential logic bug where the worker promise might hang if no tasks are present, and issues with path aliases in worker initialization. Additionally, a typo was found in a performance optimization within the texture generation logic.

Comment thread src/utils/HelperFuncs.ts
return thisData
}

const sharedBuffer = new SharedArrayBuffer(arraySize * Float16Array.BYTES_PER_ELEMENT)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

SharedArrayBuffer is only available in environments that are cross-origin isolated (requiring specific HTTP headers like COOP and COEP). In non-isolated environments, this will throw a ReferenceError. Consider adding a check for typeof SharedArrayBuffer !== 'undefined' and providing a fallback to regular ArrayBuffer or the non-worker implementation.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@lazarusA This is why it needed to be in the config

Comment thread src/utils/HelperFuncs.ts
let taskIndex = 0
let completed = 0

await new Promise<void>((resolve, reject) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

If the tasks array is empty, the workers array will also be empty, and the Promise will never resolve because the dispatch logic is only triggered for existing workers. This will cause the function to hang. You should handle the case where no tasks are generated.

  if (tasks.length === 0) {
    setStatus(null)
    return new Float16Array(sharedBuffer)
  }

  await new Promise<void>((resolve, reject) => {

Comment thread src/components/zarr/utils.ts
Comment thread src/components/textures/TextureMakers.tsx Outdated
Comment thread src/utils/HelperFuncs.ts Outdated
@TheJeran TheJeran merged commit 38b58c2 into main Apr 15, 2026
6 checks passed
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.

1 participant