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: replace fast-atomic-write with steno #285

Conversation

SgtPooki
Copy link
Member

@SgtPooki SgtPooki commented Dec 14, 2023

fixes #284

fast-write-atomic hasn't been updated in 5 years, is CJS, and is slower than steno (updated 2 months ago).

Benchmarks for various content-types & libraries (though we only use Uint8Arrays) can be found at https://github.com/SgtPooki/fast-write-atomic#benchmarks

However, there may be further room for improvement by moving to fs.createWriteStream

╰─ ✔ ❯ hyperfine --parameter-list branch 284-chore-replace-fast-write-atomic-with-steno,main --setup "git switch {branch} && npm run reset && npm i && npm run build" --runs 20  -w 1 "npm run test:node"
Benchmark 1: npm run test:node (branch = 284-chore-replace-fast-write-atomic-with-steno)
  Time (mean ± σ):     27.212 s ±  0.832 s    [User: 34.810 s, System: 6.051 s]
  Range (min … max):   25.927 s … 29.324 s    20 runs

Benchmark 2: npm run test:node (branch = main)
  Time (mean ± σ):     42.971 s ±  0.637 s    [User: 35.297 s, System: 7.534 s]
  Range (min … max):   42.178 s … 44.796 s    20 runs

Summary
  npm run test:node (branch = 284-chore-replace-fast-write-atomic-with-steno) ran
    1.58 ± 0.05 times faster than npm run test:node (branch = main)

Updated benchmarks of npm run test as of 2024-04-19

╭─    ~/code/work/protocol.ai/ipfs/js-stores    main ?1 
╰─ ✔ ❯ hyperfine --parameter-list branch main,test/not-same-event-loop-concurrency,284-chore-replace-fast-write-atomic-with-steno --setup "git switch {branch} && npm run reset && npm i && npm run build && cd packages/datastore-fs" "npm run test"
Benchmark 1: npm run test (branch = main)
  Time (mean ± σ):     99.415 s ±  2.918 s    [User: 69.659 s, System: 23.361 s]
  Range (min … max):   96.134 s … 105.200 s    10 runs

Benchmark 2: npm run test (branch = test/not-same-event-loop-concurrency)
  Time (mean ± σ):     103.456 s ±  3.186 s    [User: 74.442 s, System: 25.261 s]
  Range (min … max):   98.813 s … 108.429 s    10 runs

Benchmark 3: npm run test (branch = 284-chore-replace-fast-write-atomic-with-steno)
  Time (mean ± σ):     80.308 s ±  2.107 s    [User: 74.331 s, System: 22.228 s]
  Range (min … max):   78.219 s … 84.277 s    10 runs

Summary
  npm run test (branch = 284-chore-replace-fast-write-atomic-with-steno) ran
    1.24 ± 0.05 times faster than npm run test (branch = main)
    1.29 ± 0.05 times faster than npm run test (branch = test/not-same-event-loop-concurrency)                                                                                        [49m1.944s]

╭─    ~/code/work/protocol.ai/ipfs/js-stores    284-chore-re…c-with-steno ?1 
╰─ ✔ ❯ hyperfine --parameter-list branch main,test/not-same-event-loop-concurrency,284-chore-replace-fast-write-atomic-with-steno --setup "git switch {branch} && npm run reset && npm i && npm run build && cd packages/blockstore-fs" "npm run test"
Benchmark 1: npm run test (branch = main)
  Time (mean ± σ):     98.840 s ±  2.612 s    [User: 68.486 s, System: 22.585 s]
  Range (min … max):   97.005 s … 104.396 s    10 runs

Benchmark 2: npm run test (branch = test/not-same-event-loop-concurrency)
  Time (mean ± σ):     105.307 s ±  2.335 s    [User: 72.442 s, System: 24.766 s]
  Range (min … max):   101.167 s … 109.007 s    10 runs

Benchmark 3: npm run test (branch = 284-chore-replace-fast-write-atomic-with-steno)
  Time (mean ± σ):     77.012 s ±  1.829 s    [User: 74.442 s, System: 21.938 s]
  Range (min … max):   75.258 s … 80.825 s    10 runs

Summary
  npm run test (branch = 284-chore-replace-fast-write-atomic-with-steno) ran
    1.28 ± 0.05 times faster than npm run test (branch = main)
    1.37 ± 0.04 times faster than npm run test (branch = test/not-same-event-loop-concurrency)

@SgtPooki SgtPooki linked an issue Dec 14, 2023 that may be closed by this pull request
packages/blockstore-fs/src/index.ts Outdated Show resolved Hide resolved
packages/blockstore-fs/src/index.ts Outdated Show resolved Hide resolved
packages/blockstore-fs/src/index.ts Outdated Show resolved Hide resolved
@ipfs ipfs deleted a comment from welcome bot Dec 15, 2023
@SgtPooki SgtPooki self-assigned this Jan 16, 2024
@SgtPooki SgtPooki changed the base branch from main to test/not-same-event-loop-concurrency April 19, 2024 18:17
Copy link
Member Author

@SgtPooki SgtPooki left a comment

Choose a reason for hiding this comment

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

self review after removing cached writers. code change is much smaller now @achingbrain

@SgtPooki SgtPooki requested review from achingbrain and removed request for achingbrain April 19, 2024 20:18
@SgtPooki
Copy link
Member Author

@achingbrain anything else needed for this one or can I merge it?

Copy link
Member

@achingbrain achingbrain left a comment

Choose a reason for hiding this comment

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

LGTM, just the question over the error detection we used to do for fast-write-atomic.

@achingbrain achingbrain changed the title feat: replace fast-atomic-write with steno fix: replace fast-atomic-write with steno Aug 9, 2024
@achingbrain achingbrain merged commit c6df8dd into test/not-same-event-loop-concurrency Aug 9, 2024
19 checks passed
@achingbrain achingbrain deleted the 284-chore-replace-fast-write-atomic-with-steno branch August 9, 2024 12:02
SgtPooki added a commit that referenced this pull request Sep 17, 2024
* tmp: testing concurrent writes with worker threads

* test: add separate-thread concurrency test

* chore: re-throw errors from writer-worker

* fix: replace fast-atomic-write with steno (#285)

fixes #284

fast-write-atomic hasn't been updated in 5 years, is CJS, and is slower than steno (updated 2 months ago).

Benchmarks for various content-types & libraries (though we only use Uint8Arrays) can be found at https://github.com/SgtPooki/fast-write-atomic#benchmarks

However, there may be further room for improvement by moving to [fs.createWriteStream](https://nodejs.org/api/fs.html#fscreatewritestreampath-options)


```
╰─ ✔ ❯ hyperfine --parameter-list branch 284-chore-replace-fast-write-atomic-with-steno,main --setup "git switch {branch} && npm run reset && npm i && npm run build" --runs 20  -w 1 "npm run test:node"
Benchmark 1: npm run test:node (branch = 284-chore-replace-fast-write-atomic-with-steno)
  Time (mean ± σ):     27.212 s ±  0.832 s    [User: 34.810 s, System: 6.051 s]
  Range (min … max):   25.927 s … 29.324 s    20 runs

Benchmark 2: npm run test:node (branch = main)
  Time (mean ± σ):     42.971 s ±  0.637 s    [User: 35.297 s, System: 7.534 s]
  Range (min … max):   42.178 s … 44.796 s    20 runs

Summary
  npm run test:node (branch = 284-chore-replace-fast-write-atomic-with-steno) ran
    1.58 ± 0.05 times faster than npm run test:node (branch = main)
```

---

### Updated benchmarks of `npm run test` as of 2024-04-19

```
╭─    ~/code/work/protocol.ai/ipfs/js-stores    main ?1 
╰─ ✔ ❯ hyperfine --parameter-list branch main,test/not-same-event-loop-concurrency,284-chore-replace-fast-write-atomic-with-steno --setup "git switch {branch} && npm run reset && npm i && npm run build && cd packages/datastore-fs" "npm run test"
Benchmark 1: npm run test (branch = main)
  Time (mean ± σ):     99.415 s ±  2.918 s    [User: 69.659 s, System: 23.361 s]
  Range (min … max):   96.134 s … 105.200 s    10 runs

Benchmark 2: npm run test (branch = test/not-same-event-loop-concurrency)
  Time (mean ± σ):     103.456 s ±  3.186 s    [User: 74.442 s, System: 25.261 s]
  Range (min … max):   98.813 s … 108.429 s    10 runs

Benchmark 3: npm run test (branch = 284-chore-replace-fast-write-atomic-with-steno)
  Time (mean ± σ):     80.308 s ±  2.107 s    [User: 74.331 s, System: 22.228 s]
  Range (min … max):   78.219 s … 84.277 s    10 runs

Summary
  npm run test (branch = 284-chore-replace-fast-write-atomic-with-steno) ran
    1.24 ± 0.05 times faster than npm run test (branch = main)
    1.29 ± 0.05 times faster than npm run test (branch = test/not-same-event-loop-concurrency)                                                                                        [49m1.944s]

╭─    ~/code/work/protocol.ai/ipfs/js-stores    284-chore-re…c-with-steno ?1 
╰─ ✔ ❯ hyperfine --parameter-list branch main,test/not-same-event-loop-concurrency,284-chore-replace-fast-write-atomic-with-steno --setup "git switch {branch} && npm run reset && npm i && npm run build && cd packages/blockstore-fs" "npm run test"
Benchmark 1: npm run test (branch = main)
  Time (mean ± σ):     98.840 s ±  2.612 s    [User: 68.486 s, System: 22.585 s]
  Range (min … max):   97.005 s … 104.396 s    10 runs

Benchmark 2: npm run test (branch = test/not-same-event-loop-concurrency)
  Time (mean ± σ):     105.307 s ±  2.335 s    [User: 72.442 s, System: 24.766 s]
  Range (min … max):   101.167 s … 109.007 s    10 runs

Benchmark 3: npm run test (branch = 284-chore-replace-fast-write-atomic-with-steno)
  Time (mean ± σ):     77.012 s ±  1.829 s    [User: 74.442 s, System: 21.938 s]
  Range (min … max):   75.258 s … 80.825 s    10 runs

Summary
  npm run test (branch = 284-chore-replace-fast-write-atomic-with-steno) ran
    1.28 ± 0.05 times faster than npm run test (branch = main)
    1.37 ± 0.04 times faster than npm run test (branch = test/not-same-event-loop-concurrency)
```

* chore: fix lint error

---------

Co-authored-by: Alex Potsides <[email protected]>
github-actions bot pushed a commit that referenced this pull request Sep 17, 2024
## [blockstore-fs-v2.0.2](blockstore-fs-2.0.1...blockstore-fs-2.0.2) (2024-09-17)

### Tests

* add separate-thread concurrency test ([#305](#305)) ([5e3114e](5e3114e)), closes [#285](#285) [#284](#284)
github-actions bot pushed a commit that referenced this pull request Sep 17, 2024
## [datastore-fs-v10.0.2](datastore-fs-10.0.1...datastore-fs-10.0.2) (2024-09-17)

### Tests

* add separate-thread concurrency test ([#305](#305)) ([5e3114e](5e3114e)), closes [#285](#285) [#284](#284)
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
Status: 🏃‍♀️ In Progress
Development

Successfully merging this pull request may close these issues.

feat: replace fast-write-atomic with steno
3 participants