Async fetching from HTTPStore #8

Activity

1. manzt commented on Jan 13, 2020

   

   manzt

   on Jan 13, 2020

   Last edited by manzt

   CollaboratorAuthor

   More actions

   The performance benefits of this can be illustrated by adding a setTimeout in chunkGetItem:

   private async chunkGetItem<T extends TypedArray>(chunkCoords: number[], chunkSelection: DimensionSelection[], out: NestedArray<T>, outSelection: DimensionSelection[], dropAxes: null | number[]) {
       await new Promise(resolve => setTimeout(resolve, 1000)); // add waiting time of 1s for each request
   
       if (chunkCoords.length !== this._chunkDataShape.length) {
         throw new ValueError(`Inconsistent shapes: chunkCoordsLength: ${chunkCoords.length}, cDataShapeLength: ${this.chunkDataShape.length}`);
       }
   
       ...

   Fetching the store will take 1s per request vs ~1s + time until last request is fetched.
2. gzuidhof commented on Jan 13, 2020

   

   gzuidhof

   on Jan 13, 2020

   Owner

   More actions

   As I was implementing this I thought about this, and for sure this can and should be done asynchronously and was on my list of next steps, there is no race condition (as far as I can see).

   The reason I left it out in the initial version is to keep parity with the Python implementation (the synchronous nature in the python lib will make this very difficult, whereas here we can do it quite elegantly!).

   I think we should be slightly more careful as we may end up making a million requests all at once, so maybe we can wrap it in something that only opens XX simultaneous requests. For now we can just hardcode that count and make it configurable later.

   I would love to see a PR for this!

   ---

   Not completely unrelated:
   It's worth thinking about a way to communicate progress, it makes for a much better user experience in applications. Perhaps we should allow an optional callback that will be called with progress updates.
3. manzt commented on Jan 13, 2020

   

   manzt

   on Jan 13, 2020

   Last edited by manzt

   CollaboratorAuthor

   More actions

   Ah I see what you mean. Were you thinking something like this?

   // src/utils.ts
   export function chunkArray(arr: Array<any>, chunkSize: number) {
       if (chunkSize === 0) return arr;
       return Array.from(
           { length: Math.ceil(arr.length / chunkSize) },
           (_, i) => arr.slice(i * chunkSize, i * chunkSize + chunkSize)
       );
   }

   // src/core/index.ts
   for (const projs of chunkArray(Array.from(indexer.iter()), 10)) {
     await Promise.all(projs.map(proj => this.chunkGetItem(proj.chunkCoords, proj.chunkSelection, out, proj.outSelection, indexer.dropAxes))); 
   }

   Or since we want to know the total number of requests being made, perhaps change the indexer from a generator to return an array directly?

   ---

   Where do you see the callback being provided to the end-user?

   const z = await openArray(config);
   z.get([x,y,z], cb); // ?

   ---

   As an aside, I have been playing around with hosting via .zarr with HTTP/2 as it is allows multiplexing and seems promising for this application!
   https://github.com/manzt/zarr-simpleh2-demo
4. gzuidhof commented on Jan 13, 2020

   

   gzuidhof

   on Jan 13, 2020

   Owner

   More actions

   I don't think we need to explicitly chunk it, we just want to limit the amount of concurrent requests. We could use this library or one of its alternatives.

   ---

   For the callback, yes, something like that is exactly what I had in mind :). Perhaps instead we should pass in an object with the callback as a field (progressCallback?), that will leave future options/callbacks open. I can already think of one addition: concurrencyLimit, I think it makes sense for the store to pass a default value for it if it is not specified.

   For the HTTP/2 multiplexing experiment - very cool! I don't know a lot about HTTP2, but I imagine that this will be especially beneficial for high latency storage (e.g. S3/GCS).
5. manzt commented on Jan 14, 2020

   

   manzt

   on Jan 14, 2020

   CollaboratorAuthor

   More actions

   PR #11 is relevant to this thread.
6. manzt commented on Jan 15, 2020

   

   manzt

   on Jan 15, 2020

   CollaboratorAuthor

   More actions

   Feature added with #11
7. manzt

   closed this as completedon Jan 15, 2020
8. gzuidhof commented on Jan 15, 2020

   

   gzuidhof

   on Jan 15, 2020

   Owner

   More actions

   I released version 0.1.4 with this change (and I added a test in #13).
9. manzt commented on Jan 15, 2020

   

   manzt

   on Jan 15, 2020

   Last edited by manzt

   CollaboratorAuthor

   More actions

   Here is a JSFiddle with a progressbar!
   https://jsfiddle.net/fscd9b0w/2/
10. gzuidhof commented on Jan 15, 2020

    

    gzuidhof

    on Jan 15, 2020

    Owner

    More actions

    😍