Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ⚡ Double Digit Percentage Improvements. #169

Merged
merged 3 commits into from
Jun 29, 2023

Conversation

whizzzkid
Copy link
Contributor

@whizzzkid whizzzkid commented Jun 29, 2023

Improves: #167

Parallel Children at Each Level.

Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
@whizzzkid whizzzkid requested a review from a team as a code owner June 29, 2023 16:09
Comment on lines 63 to 69
const rootCID = await unixFs.addDirectory({ path: parentDirectoryName }, unixFsAddOptions)

for (const dirent of dirents) {
await Promise.all(dirents.map(async dirent => {
const path = nodePath.join(dir, dirent.name);

if (dirent.isDirectory()) {
const cid: CID = await addDir(path);
rootCID = await unixFs.cp(cid, rootCID, dirent.name, { offline: true })

} else {
const cid = await addFile(path);
rootCID = await unixFs.cp(cid, rootCID, dirent.name, { offline: true })
}
}
const cid: CID = dirent.isDirectory() ? await addDir(path) : await addFile(path);
await unixFs.cp(cid, rootCID, dirent.name, { offline: true })
}));
Copy link
Member

Choose a reason for hiding this comment

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

@whizzzkid since you're not changing the rootCID, the final returned rootCID is an empty directory with only a single block.

Copy link
Member

Choose a reason for hiding this comment

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

The top is using the old code, buttom is using yours:

image

@whizzzkid whizzzkid changed the title fix: ⚡ 2x Faster? fix: ⚡ Double Digit Percentage Improvements. Jun 29, 2023
@whizzzkid whizzzkid requested a review from SgtPooki June 29, 2023 18:29
@SgtPooki
Copy link
Member

pushed up some changes to this code. it does improve things, heres some output:


testing /benchmarks/add-dir/node_modules

New code

╰─ ✘ INT ❯ TEST_PATH=node_modules MIN_TIME=1000 npm start

> benchmarks-add-dir@1.0.0 start
> npm run build && node dist/src/index.js


> benchmarks-add-dir@1.0.0 build
> aegir build --bundle false

[11:28:07] tsc [started]
[11:28:08] tsc [completed]
┌─────────┬────────────────────────────┬─────────┬──────────┬──────┬───────────┬───────────────────────────────────────────────────────────────┬──────────┐
│ (index) │       Implementation       │  ops/s  │  ms/op   │ runs │    p99    │                              CID                              │  Bytes   │
├─────────┼────────────────────────────┼─────────┼──────────┼──────┼───────────┼───────────────────────────────────────────────────────────────┼──────────┤
│    0    │ 'helia-fs - node_modules'  │ '1.97'  │ '507.13' │  5   │ '2300.67' │ 'bafybeifcko3yrqn4rblklqnun64lus2vypmsejp2cvolnj35jb2sjgdcju' │ '253194' │
│    1    │ 'helia-mem - node_modules' │ '26.94' │ '37.12'  │  27  │  '64.10'  │ 'bafybeifcko3yrqn4rblklqnun64lus2vypmsejp2cvolnj35jb2sjgdcju' │ '253194' │
└─────────┴────────────────────────────┴─────────┴──────────┴──────┴───────────┴───────────────────────────────────────────────────────────────┴──────────┘

Old code

╰─ ✔ ❯ TEST_PATH=node_modules MIN_TIME=1000 npm start

> benchmarks-add-dir@1.0.0 start
> npm run build && node dist/src/index.js


> benchmarks-add-dir@1.0.0 build
> aegir build --bundle false

[11:29:25] tsc [started]
[11:29:26] tsc [completed]
┌─────────┬────────────────────────────┬─────────┬──────────┬──────┬───────────┬───────────────────────────────────────────────────────────────┬──────────┐
│ (index) │       Implementation       │  ops/s  │  ms/op   │ runs │    p99    │                              CID                              │  Bytes   │
├─────────┼────────────────────────────┼─────────┼──────────┼──────┼───────────┼───────────────────────────────────────────────────────────────┼──────────┤
│    0    │ 'helia-fs - node_modules'  │ '1.02'  │ '984.31' │  5   │ '4491.49' │ 'bafybeifcko3yrqn4rblklqnun64lus2vypmsejp2cvolnj35jb2sjgdcju' │ '253194' │
│    1    │ 'helia-mem - node_modules' │ '18.45' │ '54.19'  │  19  │  '88.45'  │ 'bafybeifcko3yrqn4rblklqnun64lus2vypmsejp2cvolnj35jb2sjgdcju' │ '253194' │
└─────────┴────────────────────────────┴─────────┴──────────┴──────┴───────────┴───────────────────────────────────────────────────────────────┴──────────┘

Results

helia-fs -- ms/op decrease of 48.48%
helia-mem -- ms/op decrease of 31.5%

testing /node_modules/ipfs-core

New Code

╰─ ✔ ❯ TEST_PATH=../../node_modules/ipfs-core MIN_TIME=1000 npm start

> benchmarks-add-dir@1.0.0 start
> npm run build && node dist/src/index.js


> benchmarks-add-dir@1.0.0 build
> aegir build --bundle false

[11:32:41] tsc [started]
[11:32:42] tsc [completed]
┌─────────┬────────────────────────────────────────────┬────────┬───────────┬──────┬────────────┬───────────────────────────────────────────────────────────────┬────────────┐
│ (index) │               Implementation               │ ops/s  │   ms/op   │ runs │    p99     │                              CID                              │   Bytes    │
├─────────┼────────────────────────────────────────────┼────────┼───────────┼──────┼────────────┼───────────────────────────────────────────────────────────────┼────────────┤
│    0    │ 'helia-fs - ../../node_modules/ipfs-core'  │ '0.15' │ '6678.48' │  5   │ '30699.80' │ 'bafybeic4duvrbtc4l5cmjnobtky5cpzkc27wjlbmqdbxdbvkpo7vlnrsue' │ '16914820' │
│    1    │ 'helia-mem - ../../node_modules/ipfs-core' │ '1.74' │ '574.72'  │  5   │  '621.87'  │ 'bafybeic4duvrbtc4l5cmjnobtky5cpzkc27wjlbmqdbxdbvkpo7vlnrsue' │ '16914820' │
└─────────┴────────────────────────────────────────────┴────────┴───────────┴──────┴────────────┴───────────────────────────────────────────────────────────────┴────────────┘

Old Code

╰─ ✔ ❯ TEST_PATH=../../node_modules/ipfs-core MIN_TIME=1000 npm start

> benchmarks-add-dir@1.0.0 start
> npm run build && node dist/src/index.js


> benchmarks-add-dir@1.0.0 build
> aegir build --bundle false

[11:31:24] tsc [started]
[11:31:25] tsc [completed]
┌─────────┬────────────────────────────────────────────┬────────┬───────────┬──────┬────────────┬───────────────────────────────────────────────────────────────┬────────────┐
│ (index) │               Implementation               │ ops/s  │   ms/op   │ runs │    p99     │                              CID                              │   Bytes    │
├─────────┼────────────────────────────────────────────┼────────┼───────────┼──────┼────────────┼───────────────────────────────────────────────────────────────┼────────────┤
│    0    │ 'helia-fs - ../../node_modules/ipfs-core'  │ '0.12' │ '8094.28' │  5   │ '35748.45' │ 'bafybeic4duvrbtc4l5cmjnobtky5cpzkc27wjlbmqdbxdbvkpo7vlnrsue' │ '16914820' │
│    1    │ 'helia-mem - ../../node_modules/ipfs-core' │ '1.29' │ '776.65'  │  5   │  '824.25'  │ 'bafybeic4duvrbtc4l5cmjnobtky5cpzkc27wjlbmqdbxdbvkpo7vlnrsue' │ '16914820' │
└─────────┴────────────────────────────────────────────┴────────┴───────────┴──────┴────────────┴───────────────────────────────────────────────────────────────┴────────────┘

Results

helia-fs -- ms/op decrease of 17.49%
helia-mem -- ms/op decrease of 35.14%

@whizzzkid whizzzkid merged commit e0db228 into benchmark/add-dir Jun 29, 2023
@whizzzkid whizzzkid deleted the fix/benchmark/add-dir branch June 29, 2023 18:39
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 <1895906+whizzzkid@users.noreply.github.com>

* fix: dag generation with parallelization at each level

---------

Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
Co-authored-by: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com>

* 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 <1895906+whizzzkid@users.noreply.github.com>
Co-authored-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
Co-authored-by: Alex Potsides <alex@achingbrain.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

2 participants