rvagg / js-ipld-hashmap

An associative array Map-type data structure for very large, distributed data sets built on IPLD
Other
23 stars 4 forks source link

skip block codec if store is Block aware #53

Open inverted-capital opened 1 year ago

inverted-capital commented 1 year ago

As mentioned in #32 this is a patch for allowing us to work with blocks directly, which has increased our performance somewhat by avoiding the double encoding.

Please advise if another method of achieving this is preferrable.

rvagg commented 1 year ago

Yeah, this is pretty nice actually. But .. you have a typescript problem to deal with now. See https://github.com/rvagg/js-ipld-hashmap/blob/fd113adb6dce5f8c0b12d089f8ad8d278725f912/interface.ts#L45-L48 where the loader is defined. How much expertise do you have with TS and are you confident enough to make a BlockLoader interface and make it accept a Loader|BlockLoader where it currently accepts a Loader? That's my suggestion for dealing with this anyway. I don't think it'd be reasonable to make a Loader require all 4 functions—although, one thing we could do is when accepting a Loader initially, to convert it to a BlockLoader internally so the code you've modified here only ever needs to deal with getBlock and putBlock and the Block stuff is abstracted away nicely.

inverted-capital commented 1 year ago

Call me ghetto but I don't actually use TypeScript (yet !) - I don't know how to make the interface changes you mention. I can take a crack at it, but it might come out pretty butchered !

rvagg commented 1 year ago

Yeah, fair enough, I'm not exactly a TS user myself although I'm heavily using TS annotations throughout JS code to do this kind of typechecking. Here's a slightly more complicated interface definition that might be helpful: https://github.com/rvagg/cborg/blob/master/interface.ts.

You'll likely need to duplicate Loader and call it BlockLoader, change the names and make it deal with Block objects, which you should be able to import { Block } from 'multiformats/block/interface'. Then where Loader is accepted (in the jsdoc inline definitions), accept a Loader|BlockLoader. Or you can do the thing where there's a function that takes a Loader|BlockLoader and returns a BlockLoader which is the only thing that's used from there on.

inverted-capital commented 1 year ago

Ok thank you for the tips - also thank you for the libraries too - they are super useful to us