lmdb: Enable MDB_USE_PWRITEV on Linux

[lmb]

This is a small optimization, which turns a seek + writev into a pwritev syscall on Linux.

[bmatsuo]

Oh interesting. I didn't realize that this system call was guarded by a #define.

Are there times when pwritev will actually hurt performance? It doesn't sound like it would. But I don't actually have much experience with the call. And right now I'm not sure why it would be placed behind a flag.

One of the few times I have seen the call discussed is in https://github.com/boltdb/bolt/issues/238. But that seems more like it wasn't implemented due to constrained resources than potential issues.

I will benchmark, look up anything I can find it the mailing list, and get back here.

Thanks

[bmatsuo]

I don't really see a change in performance. Do you actually see any change if you run the benchmarks or in any programs you have?

benchmark                              old ns/op     new ns/op     delta
BenchmarkEnv_ReaderList-4              51130         51319         +0.37%
BenchmarkTxn_Put-4                     1526          1530          +0.26%
BenchmarkTxn_PutReserve-4              1610          1611          +0.06%
BenchmarkTxn_PutReserve_writemap-4     1344          1335          -0.67%
BenchmarkTxn_Put_writemap-4            1278          1290          +0.94%
BenchmarkTxn_Get_ro-4                  1398          1416          +1.29%
BenchmarkTxn_Get_raw_ro-4              783           798           +1.92%
BenchmarkScan_ro-4                     5890150       5901033       +0.18%
BenchmarkScan_raw_ro-4                 1521376       1527676       +0.41%
BenchmarkCursor-4                      691           687           -0.58%
BenchmarkCursor_Renew-4                179           178           -0.56%
BenchmarkTxn_Sub_commit-4              189910        188408        -0.79%
BenchmarkTxn_Sub_abort-4               209512        184603        -11.89%
BenchmarkTxn_abort-4                   14391         14075         -2.20%
BenchmarkTxn_commit-4                  14413         14634         +1.53%
BenchmarkTxn_ro-4                      144813        161165        +11.29%
BenchmarkTxn_unmanaged_abort-4         14490         14374         -0.80%
BenchmarkTxn_unmanaged_commit-4        14510         14517         +0.05%
BenchmarkTxn_unmanaged_ro-4            149717        147595        -1.42%
BenchmarkTxn_renew-4                   532           528           -0.75%
BenchmarkTxn_Put_append-4              468           465           -0.64%
BenchmarkTxn_Put_append_noflag-4       619           653           +5.49%

The benchmarks above that really swing are unrelated, I think. They should not be flushing pages. And they are generally quite noisy.

I also haven't been able to find any discussion of the flag in the official mailing list. I may have missed something. But I'm really at a loss about this flag. Perhaps I don't have the appropriate use case in the benchmarks. In general I think the benchmarks could be improved and more focused. But they do provide some utility.

[mranney]

I'm surprised that this doesn't help. Seems like the cutting the number of system calls in half should be an obvious win. Maybe the writes in the bench loop are so fast as it is that the difference ends up not showing up compared to whatever else is going on.

[bmatsuo]

I have started writing some benchmarks which I believe will make the impact more apparent. And what I've seen so far makes me hopeful.

Looking over how pwritev gets used. It seems that you would see the flag's benefits when committing multiple writes (on multiple pages) per transaction. I don't think that this is being benchmarked effectively on master.

[bmatsuo]

@mranney furthermore, to your point about it being an obvious win, I would absolutely agree with you taking the system call and LMDB code at face value. The thing that confuses me is why LMDB will not try define this for linux systems by default.

The C Makefile states

Note that the defaults [of MDB_USE_PWRITEV] should already be correct on most platforms; you should not need to change [its value].

AFAICT the library then makes no attempt to define the flag for linux systems.

[lmb]

My unsubstantiated guess is that the flag is set in the OpenLDAP Makefile, but I have not checked yet. On 13 Mar 2016 11:54 am, "Bryan Matsuo" notifications@github.com wrote:

@mranney https://github.com/mranney furthermore, to your point about it being an obvious win, I would absolutely agree with you taking the system call and LMDB code at face value. The thing that confuses me is why LMDB will not try define this for linux systems by default.

The C Makefile states

Note that the defaults [of MDB_USE_PWRITEV] should already be correct on most platforms; you should not need to change [its value].

AFAIK the library then make no attempt to define the flag for linux systems.

— Reply to this email directly or view it on GitHub https://github.com/bmatsuo/lmdb-go/pull/58#issuecomment-195941833.

[bmatsuo]

@lmb AFAICT that is not the case. It looks like there is a "grep" interface for the openldap repo.

http://www.openldap.org/devel/gitweb.cgi?p=openldap.git&a=search&h=HEAD&st=grep&s=PWRITEV

Searching for "PWRITEV" only turns up known references in the liblmdb tree (edit: subtree).

[bmatsuo]

I've put the question to the openldap-technical mailing list. A definitive answer should come from there. I will link the response here from the web archives after I have received it.

[bmatsuo]

Sorry for not posting an update. But I never got a response from the openldap-technical mailing list. So I'm still a little confused there.

But it has come to my attention that the minimum system requirements for Go on Linux are kernel version 2.6.23. I was previously under the impression that the minimum required version was 2.6.32 (oops)

https://golang.org/doc/install#requirements

The pwrite(2) system call wasn't introduced until 2.6.30. So I'm somewhat concerned about people not being able to build the package if they have an old kernel.

I don't use custom build tags much. But would it be practical to put the #CGO linux CFLAGS in a separate file that is guarded by some special build tag like pwritev?

//+build pwritev

package lmdb

/*
CGO linux CFLAGS: -DMDB_USE_PWRITEV
*/
import C

That might not be completely correct syntax, but it is otherwise possible to define CFLAGS in multiple places AFAIK. I just don't really know the details of how to build non-main packages with custom build tags.

---

On another note I haven't made significant progress on benchmarks. I actually hate writing benchmarks for lmdb-go. While the current ones are especially bad it is impossible to completely remedy because they always be inherently artificial.

[lmb]

Good catch re the system requirements. I think what you propose is possible by simply adding the build tag to the CGO comment (where it currently says linux). Consumers of the library can then specify the build tag when invoking go build or go install.

For benchmarks, I think that CGO is probably the main offender here. The special scheduling alone will make it hard to get consistent results. Especially with such a small change as this (which could be a good indicator that its not worth worrying about this in a world of CGO overheads). On 23 Mar 2016 7:40 pm, "Bryan Matsuo" notifications@github.com wrote:

Sorry for not posting an update. But I never got a response from the openldap-technical mailing list. So I'm still a little confused there.

But it has come to my attention that the minimum system requirements for Go on Linux are kernel version 2.6.23. I was previously under the impression that the minimum required version was 2.6.32 (oops)

https://golang.org/doc/install#requirements

The pwrite(2) system call wasn't introduced until 2.6.30. So I'm somewhat concerned about people not being able to build the package if the have an old kernel.

I don't use custom build tags much. But would it be practical to put the #CGO linux CFLAGS in a separate file that is guarded by some special build tag like pwritev?

//+build pwritev

package lmdb

/ CGO linux CFLAGS: -DMDB_USE_PWRITEV / import C

That might not be completely correct syntax, but there is otherwise possible to define CFLAGS in multiple places AFAIK. I just don't really

know the details of how to build non-main packages with custom build tags.

On another note I haven't made significant progress on benchmarks. I actually hate writing benchmarks for lmdb-go. While the current ones are especially bad it is impossible to completely remedy because they always be inherently artificial.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/bmatsuo/lmdb-go/pull/58#issuecomment-200513633

[bmatsuo]

Those seem like good observations about CGO. Honestly I think a real application could see benefits regardless of CGO depending on its use case for LMDB. But it's so wishy-washy that they would need actually benchmark their apps on real data to confirm. YMMV, absolutely.

One critical problem with the benchmarks is that I don't want them to take 10+ minutes to run every time. And on top of that I don't want them to take up 20GB of disk space per benchmark. So it is difficult to confirm those kinds of assertions in lmdb-go itself.

If a build tag is possible to work with then I think that is the correct way forward here. I was a little wary when I saw that go get will not allow build tag specification. But if it possible for an application to recompile it later with a build tag then that sounds ok.

[lmb]

I've updated the PR with a new build tag and some basic doc on how to use it.

[bmatsuo]

This looks great. Thanks.

The travis-ci build broke for dumb reasons. But only go1.4 failed for known reasons. I am just going to merge this.

Thanks again.