test: use globSource in add-dir benchmark #171
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Use `globSource` from ipfs-utils for import - Make all impls return the same CID - Remove the dir size - if the CID is the same that's good enough
Member
Author
|
Before: After: |
achingbrain
changed the title
Member
Author
|
The kubo-direct benchmarks are wildly different which is not something I expected to see. I'm not 100% convinced by tinybench, I think the test cases interfere with each other. That is, I've got different results in benchmarks by running them in a different order 🙃 |
whizzzkid
approved these changes
Jul 5, 2023
Contributor
whizzzkid
left a comment
whizzzkid
left a comment
There was a problem hiding this comment.
nice, good improvements! minor nits!
| nodePath.relative(process.cwd(), nodePath.join(process.cwd(), '..', 'gc', 'src')), | ||
| ] | ||
| const testPaths = TEST_PATH != null | ||
| ? [TEST_PATH] |
| }) | ||
|
|
||
| const addFile = async (path: string) => (await controller.api.add({ path: nodePath.relative(process.cwd(), path), content: fs.createReadStream(path)}, { cidVersion: 1, pin: false })).cid | ||
| const addFile = async (path: string): Promise<CID> => (await controller.api.add({ path: nodePath.relative(process.cwd(), path), content: fs.createReadStream(path) }, { cidVersion: 1, pin: false })).cid |
Contributor
There was a problem hiding this comment.
can we split this in a meaningful way to make this more readable?
Member
Author
There was a problem hiding this comment.
I'm not sure what you mean, it's as readable as it was before, the only change on this line is an automated linting fix.
That said, having it as a one-liner isn't very readable so I do agree it can be improved - can you suggest an edit instead?
Member
Author
There was a problem hiding this comment.
Though maybe in #167 - it's probably out of scope here.
Member
We could probably remove kubo-direct, and we should probably use a more reliable benchmark framework (or restructure these benchmarks to be more reliable) if we're pointing people to these. |
SgtPooki
approved these changes
Jul 6, 2023
| addFile, | ||
| addDir, | ||
| // TODO: Fix timing out during size calculation. | ||
| // getSize: getFolderSize |
Member
There was a problem hiding this comment.
My intention behind the size was to confirm all expected files/folders were read during the benchmark tests.
If we get the same CID from all implementations, we could cat one of them to confirm it's what we expect.
I think we should have some sort of validation but maybe we can handle that when we handle #170
SgtPooki
added a commit
that referenced
this pull request
Jul 11, 2023
* test: create add-dir benchmark * chore(bench/add-dir): remove unused taskResult prop * test(bench/add-dir): add kubo-direct test * docs(bench/add-dir): ipfs-core node_modules output * chore(bench/add-dir): kubo-direct doesnt pin files when adding * chore(bench/add-dir): split helia benchmark by blockstore type (fs/mem) * fix: ⚡ Double Digit Percentage Improvements. (#169) * fix: ⚡ 2x improvement Signed-off-by: Nishant Arora <[email protected]> * fix: dag generation with parallelization at each level --------- Signed-off-by: Nishant Arora <[email protected]> Co-authored-by: Russell Dempsey <[email protected]> * test: use globSource in add-dir benchmark (#171) * fix: use glob source for importing directories - Use `globSource` from ipfs-utils for import - Make all impls return the same CID - Remove the dir size - if the CID is the same that's good enough * chore: fix linting * chore: remove benchmark from workspaces * test: align error and success output headers --------- Signed-off-by: Nishant Arora <[email protected]> Co-authored-by: Nishant Arora <[email protected]> Co-authored-by: Alex Potsides <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
globSourcefrom ipfs-utils for import