Skip to content

skip block codec if store is Block aware#53

Open
inverted-capital wants to merge 3 commits into
rvagg:masterfrom
inverted-capital:master
Open

skip block codec if store is Block aware#53
inverted-capital wants to merge 3 commits into
rvagg:masterfrom
inverted-capital:master

Conversation

@inverted-capital

Copy link
Copy Markdown

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.

Comment thread ipld-hashmap.js Outdated
Comment thread ipld-hashmap.js Outdated
@rvagg

rvagg commented Jan 10, 2023

Copy link
Copy Markdown
Owner

Yeah, this is pretty nice actually. But .. you have a typescript problem to deal with now. See

export interface Loader {
get (cid: CID): Promise<Uint8Array>
put (cid: CID, bytes: Uint8Array): Promise<void>
}
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 and others added 2 commits January 10, 2023 16:04
Co-authored-by: Rod Vagg <rod@vagg.org>
Co-authored-by: Rod Vagg <rod@vagg.org>
@inverted-capital

Copy link
Copy Markdown
Author

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

rvagg commented Jan 10, 2023

Copy link
Copy Markdown
Owner

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

Copy link
Copy Markdown
Author

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

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.

2 participants