posix: optimize small writes

[marcinslusarz]

This patchset replaces algorithm using pmemobj transactions with one which uses transactions only for space allocation and updates the rest (mtime, ctime, size) using atomic algorithm based on versioning.

On emulated pmem (using DRAM) overwrite performance increases by factor of 2.5 for 64 byte writes and 2 for 512 byte writes. Even 64k writes improve by quite a lot (22%).

Each patch can be reviewed individually. Patches 2 and 3 should be no-ops.

---

This change is 

[codecov[bot]]

Codecov Report

Merging #393 into master will increase coverage by 0.16%. The diff coverage is 87.07%.

@@            Coverage Diff             @@
##           master     #393      +/-   ##
==========================================
+ Coverage   77.01%   77.17%   +0.16%     
==========================================
  Files          67       67              
  Lines        8345     8466     +121     
  Branches     1684     1694      +10     
==========================================
+ Hits         6427     6534     +107     
- Misses       1500     1506       +6     
- Partials      418      426       +8

Flag	Coverage Δ
#ltp_tests	47.39% <77.55%> (+1.04%)	:arrow_up:
#sqlite_tests	46.05% <75.51%> (+0.91%)	:arrow_up:
#tests_antool	15% <0%> (-0.22%)	:arrow_down:
#tests_posix_multi_threaded	26.17% <62.11%> (+1.16%)	:arrow_up:
#tests_posix_single_threaded	54.83% <84.35%> (+0.51%)	:arrow_up:
#tests_preload	45.42% <72.78%> (+0.71%)	:arrow_up:

Impacted Files	Coverage Δ
src/libpmemfile-posix/truncate.c	90% <100%> (-0.27%)	:arrow_down:
src/libpmemfile-posix/lseek.c	86.09% <100%> (+1.42%)	:arrow_up:
src/libpmemfile-posix/unlink.c	95% <100%> (+0.06%)	:arrow_up:
src/libpmemfile-posix/timestamps.c	91.61% <100%> (-0.11%)	:arrow_down:
src/libpmemfile-posix/inode.h	100% <100%> (ø)	:arrow_up:
src/libpmemfile-posix/read.c	94.59% <100%> (+0.03%)	:arrow_up:
src/libpmemfile-posix/rename.c	90.32% <100%> (-0.95%)	:arrow_down:
src/libpmemfile-posix/utils.h	80% <100%> (+13.33%)	:arrow_up:
src/libpmemfile-posix/chown.c	98.97% <100%> (-0.02%)	:arrow_down:
src/libpmemfile-posix/readlink.c	100% <100%> (ø)	:arrow_up:
... and 22 more

---

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 b147ebd...953ebd3. Read the comment docs.

[marcinslusarz]

I added more comments and removed inode_slot_next, as requested by Gabor offline.

[sarahjelinek]

---

Reviewed 30 of 34 files at r1, 1 of 3 files at r2, 3 of 3 files at r3. Review status: all files reviewed at latest revision, 3 unresolved discussions.

---

Comments from Reviewable

[GBuella]

---

Reviewed 3 of 3 files at r4. Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

src/libpmemfile-posix/data.c, line 604 at r3 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

... which is calling is_offset_in_block internally.

Exactly, so we wouldn't have to repeat this pattern a lot. Why would we have that function, if we don't want to use it? And checking once before a do-while loop makes it a weird version of a while loop... or not? Am I missing something? What is that difference between these:

   blablabla...

   if (!is_offset_in_block(block, offset))
            return false;

   do {
      do_stuff...
   } while (iterator < offset + size && is_offset_in_block(block, offset));

   blablabla...

   while (iterator < offset + size && is_offset_in_block(block, offset)) {
     do_stuff...
   }

The first one does the iterator < offset + size comparison one more time, that is a problem?

---

Comments from Reviewable

[GBuella]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

src/libpmemfile-posix/data.c, line 604 at r3 (raw file):

Previously, GBuella (Gabor Buella) wrote…

Exactly, so we wouldn't have to repeat this pattern a lot. Why would we have that function, if we don't want to use it? And checking once before a do-while loop makes it a weird version of a while loop... or not? Am I missing something? What is that difference between these: ``` blablabla... if (!is_offset_in_block(block, offset)) return false; do { do_stuff... } while (iterator < offset + size && is_offset_in_block(block, offset)); ``` ``` blablabla... while (iterator < offset + size && is_offset_in_block(block, offset)) { do_stuff... } ``` The first one does the `iterator < offset + size` comparison one more time, that is a problem?

I mean "the first one does that comparison one fewer times". And I mean "is_offset_in_block(block, iterator)" not "is_offset_in_block(block, offset)" LGTM anyways, just looks more verbose than I would make it.

---

Comments from Reviewable

[krzycz]

Reviewed 26 of 34 files at r1, 1 of 3 files at r3, 2 of 3 files at r4. Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

src/libpmemfile-posix/utils.h, line 47 at r4 (raw file):

#define pmemfile_persist(pfp, p) pmemobj_persist((pfp)->pop, (p), sizeof(*(p)))

static inline void

just out of curiosity - why the above are macros and this one is an inline function?

---

Comments from Reviewable

[GBuella]

Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

src/libpmemfile-posix/utils.h, line 47 at r4 (raw file):

Previously, krzycz (Krzysztof Czurylo) wrote…

just out of curiosity - why the above are macros and this one is an inline function?

Because I was nagging Marcin with my dislike of preprocessor macro cowboy tricks, and this pmemfile_drain in particular has no justification for being a macro -- unlike the ones above using sizeof on a type pointed to by a general pointer.

---

Comments from Reviewable

[krzycz]

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

---

Comments from Reviewable

[marcinslusarz]

Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

src/libpmemfile-posix/data.c, line 604 at r3 (raw file):

Previously, GBuella (Gabor Buella) wrote…

I mean "the first one does that comparison one fewer times". And I mean "is_offset_in_block(block, iterator)" not "is_offset_in_block(block, iterator)" LGTM anyways, just looks more verbose than I would make it.

My version checks first whether starting offset is in any allocated block and only then checks whether [offset, offset+size] has block coverage. IMO your version is a bit harder to understand, because the loop serves both purposes.

---

Comments from Reviewable

[krzycz]

Reviewed 1 of 34 files at r1, 1 of 3 files at r2, 1 of 3 files at r3, 1 of 3 files at r4. Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

Comments from Reviewable

[krzycz]

---

Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

Comments from Reviewable

[GBuella]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

src/libpmemfile-posix/data.c, line 604 at r3 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

My version checks first whether starting offset is in any allocated block and only then checks whether [offset, offset+size] has block coverage. IMO your version is a bit harder to understand, because the loop serves both purposes.

I don't get that, how is the starting offset different from any other offset in the range?

---

Comments from Reviewable

[GBuella]

---

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Comments from Reviewable

[marcinslusarz]

Rebased.

[sarahjelinek]

---

Reviewed 1 of 19 files at r5. Review status: 16 of 34 files reviewed at latest revision, 2 unresolved discussions.

---

Comments from Reviewable