search

pmem / pmemfile

Userspace implementation of file APIs using persistent memory.
Other
34 stars 21 forks source link

posix: update atime without transaction #296

Closed igchor closed 7 years ago

igchor commented 7 years ago

This change improves performance in sqlite tests by ~15%

This change is Reviewable

marcinslusarz commented 7 years ago

"posix: update atime without transaction" is shorter and clearer IMHO.

marcinslusarz commented 7 years ago
:lgtm:

Reviewed 1 of 1 files at r1. Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

krzycz commented 7 years ago
:lgtm:

Reviewed 1 of 1 files at r1. Review status: all files reviewed at latest revision, all discussions resolved, some commit checks pending.

Comments from Reviewable

codecov-io commented 7 years ago

Codecov Report

Merging #296 into master will decrease coverage by <.01%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #296      +/-   ##
==========================================
- Coverage   79.42%   79.41%   -0.01%     
==========================================
  Files          81       81              
  Lines       12124    12123       -1     
  Branches     1641     1642       +1     
==========================================
- Hits         9629     9628       -1     
  Misses       1889     1889              
  Partials      606      606
Impacted Files Coverage Δ
src/libpmemfile-posix/read.c 94.15% <100%> (-0.12%) :arrow_down:
src/libpmemfile-posix/file.c 65.93% <0%> (-0.28%) :arrow_down:
tests/posix/mt/mt.cpp 86.45% <0%> (+0.07%) :arrow_up:
src/libpmemfile-posix/rename.c 91.49% <0%> (+0.4%) :arrow_up:
src/libpmemfile-posix/utils.c 73.43% <0%> (+0.42%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update db0b69d...6475ed0. Read the comment docs.

krzycz commented 7 years ago

:+1:

marcinslusarz commented 7 years ago

Sub-second timestamps are useful. Unless there's a very good reason I would like to keep them.

However you raised a good point - we may get inconsistent data if power failure would occur between seconds and nanoseconds flush, so we can't merge this patch as is. It needs to wait for proper atomic algo.

marcinslusarz commented 7 years ago
:lgtm_cancel:

Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.

Comments from Reviewable

sarahjelinek commented 7 years ago

Let's delete this pull request since it isn't ready for merging.