LevelDB 1.20 #10138

Closed
fanquake opened this Issue Apr 2, 2017 · 2 comments

Comments

Projects
None yet
2 participants
Contributor

fanquake commented Apr 2, 2017 edited

  • - Pull LevelDB 1.20 into the upstream bitcoin-core/levedb repo. Currently at 1.19
  • - Update this repository with LevelDB 1.20. It is also currently at 1.19.
  • - Write release notes portion outlining LevelDB performance improvements.

LevelDB 1.20 release notes

Convert documentation to markdown.
Implement support for Intel crc32 instruction (SSE 4.2). Based on #309.
Limit the number of read-only files the POSIX Env will have open.
Add option for maximum file size.

related IRC chat

15:46:11 sipa: wumpus: btw, LevelDB .20 adds hw accelerated crc for intel
15:46:30 gmaxwell: \O/
15:48:13 wumpus: sipa: good to know
15:49:12 wumpus: yep https://github.com/google/leveldb/blob/master/port/port_posix_sse.cc
15:49:27 wumpus: that will really help our performance (on supported hw0
15:50:35 gmaxwell: should probably do that upgrade sooner rather than later in 0.15 to give time to find any compatiblity issues.
15:50:35 sipa: we may want to add arm code to do the same ourselves
15:51:20 wumpus: indeed
15:57:32 wumpus: yes would make sense to bump our leveldb in master to that soon
15:58:30 sipa: we'll need some build process integration to detect and pass down the SSE defines
15:58:37 sipa: as we bypass their build system
15:58:44 wumpus: would also get https://github.com/bitcoin-core/leveldb/pull/17
15:59:05 wumpus: isn't the SSE stuff at runtime?
15:59:12 wumpus: I'd hope so 
15:59:39 gmaxwell: at least on intel runtime autodetection is simple-- and as I've mentioned before, recent g++ has a sensible looking function overrides based way of doing the substitution.
15:59:56 gmaxwell: but leveldb didn't implement runtime? lameo. 
16:00:06 sipa: yes, they detect at runtime
16:00:08 wumpus: they for fact have runtime detection, I saw HaveSSE42 in the file I linked
16:00:20 wumpus: dunno if they use it
16:00:26 sipa: but you still need a compiler that has support for it
16:01:28 wumpus: "In a separate source file to allow this accelerated CRC32C function to be compiled with the appropriate compiler flags to enable x86 SSE 4.2instructions."
16:01:31 wumpus: indeed
16:01:42 wumpus: that specific file needs special compiler flags
16:02:28 sipa: they use intel instrinsics in gcc, not native assembly
16:03:30 gmaxwell: yea thats a pretty normal approach, you have one file compiled with -msse4.2 and dispatch elsewhere calls into it only on the right hardware.
16:04:22 gmaxwell: I think some of the examples for the function overriding stuff used pragmas to push the compiler flags just for that function, so if I'm not misremembering apparently that works.
16:47:07 wumpus: cfields: do you have time to look at integrating the leveldb sse42 stuff into our build system? if not, no problem I'll take a look at it, it doesn't seem particularly difficult though I wouldn't know the autoconf/automake incantation for compiling a file with different flags
16:47:41 sipa: i think i can do it
16:48:06 sipa: ah, nvm, you only want that single file to have -msse4
16:48:25 wumpus: yep, that's the challenge 
16:48:27 sipa: that i fon't know
16:48:30 sipa: *don't
16:50:55 sipa: probably needs a separate .a file?
16:51:01 wumpus: sse4.2 isn't yet widely available enough to compile everything with it, unlike sse2 for example which every 64-bit processor has
16:51:28 wumpus: every 64-bit *x86* processor sorry 
16:51:50 wumpus: sipa: not sure, maybe the attributes can be added per .o file too
17:00:23 sipa: wumpus: as leveldb upstream does not look like they'll pick up my barrier patch anytime soon, i'll submit it to our leveldb repo instead?
17:02:07 wumpus: sipa: sounds good to me

fanquake added the Upstream label Apr 2, 2017

fanquake added this to the 0.15.0 milestone Apr 2, 2017

Contributor

fanquake commented Jun 14, 2017

Mostly completed by #10544.

MarcoFalke referenced this issue Jun 18, 2017

Open

TODO for release notes 0.15.0 #9889

0 of 10 tasks complete
Owner

laanwj commented Jul 13, 2017

I'm going to close this. Release notes mention would be nice, as part of all the performance improvements in 0.15.0, but this doesn't warrant keeping open a separate issue.

laanwj closed this Jul 13, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment