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

Do not allocate when flushing #279

Merged
merged 2 commits into from Jun 5, 2021
Merged

Do not allocate when flushing #279

merged 2 commits into from Jun 5, 2021

Conversation

pascutto
Copy link
Contributor

No description provided.

@samoht samoht added this to the Next Release milestone Apr 7, 2021
@samoht
Copy link
Member

samoht commented Apr 28, 2021

This needs to cleaning up - @pascutto will do it one day :-)

@craigfe
Copy link
Member

craigfe commented Apr 29, 2021

Rebased.

@pascutto pascutto marked this pull request as ready for review May 19, 2021 12:58
Copy link
Contributor

@icristescu icristescu left a comment

Choose a reason for hiding this comment

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

Looks good, thanks! can you add some memtrace graphs to track the effect of this PR?

src/unix/raw.ml Outdated Show resolved Hide resolved
src/unix/raw.ml Outdated Show resolved Hide resolved
pascutto and others added 2 commits June 5, 2021 23:04
This value wasn't provided in the standard library until 4.09.
@craigfe
Copy link
Member

craigfe commented Jun 5, 2021

Thanks! Merging.

This is a very significant performance improvement for write-intensive loads. Running benchmarks with:

; dune exec bench/bench.exe -- --nb-entries 10_000_000 --filter '^replace_random$' --log-size 200_000

For this benchmark, this PR reduces total allocations from 35G to 24G, and makes memory usage more stable. Before:

image

After:

image

@craigfe craigfe merged commit 65666c0 into mirage:master Jun 5, 2021
craigfe added a commit to craigfe/opam-repository that referenced this pull request Jun 16, 2021
CHANGES:

## Fixed

- Fixed a crash-consistency bug due to a potential flush of an incomplete entry
  to disk. Entries are now flushed as complete strings. (mirage/index#301)

- Fixed a performance issue for `Index.sync` when there is a blocking merge in
  progress: the `log_async` file was not cached properly and fully reloaded
  from disk every time. (mirage/index#310)

- Added fsync after `Index.clear` to signal more quickly to read-only instances
  than something has changed in the file (mirage/index#308)

- Attempt to recover from `log_async` invariant violations during an explicit
  sync operation, rather than failing immediately. (mirage/index#329)

## Changed

- Release overly defensive warnings occuring when pre-fetching the disk. (mirage/index#322)

- Specialise `IO.v` to create read-only or read-write instances. (mirage/index#291)

- Optimised the in-memory representation of index handles and intermediate
  buffers, resulting in a significant reduction in memory use. (mirage/index#273, mirage/index#279)

- Benches are now executed 3 times and a new option `nb-exec` has been added (mirage/index#292)

- `clear` removes the files on disks and opens new ones containing only the
  header. (mirage/index#288)

- `Index.Make` now requires an implementation of a monotonic time source.
  (mirage/index#321)

- The `Index.Make` functor now takes a single `Platform` argument containing
  all system dependencies (i.e. `IO`, `Clock`, `Semaphore` and `Thread`). The
  `Platform` module holds the necessary types for these modules. (mirage/index#321, mirage/index#330)

## Added

- Added benchmarks that replay a trace of index operations. (mirage/index#300)

- Log reporter for the benches
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.

None yet

4 participants