IRC meeting summary for 2018-08-02

Overview


Topics discussed during this weekly meeting included how to deal with conflicting compiler flags for a particular edge case, continued discussion from last week about fixing a problem with unicode in filenames for Windows users, and changing how the program (particularly its database component) opens files on some systems.

Briefly before the start of topics, Wladimir van der Laan linked to the pending PRs, pending issues, and planned release schedule for Bitcoin Core 0.17. Potential contributors are encouraged to review these pages and look ways to help move the release forward.

CXXFLAGS stuff

Background: the scripts used to build Bitcoin Core pass parameters (flags) to the compiler to tell it what resources Bitcoin Core needs and which optimizations to use or avoid. Recently, this included adding a -mavx2 flag (Mode AVX2) to enable hardware acceleration for SHA256 hashing on supported CPUs. In addition to the flags Bitcoin Core passes, users can pass additional parameters using the CXXFLAGS variable, including the flag -mno-avx2 (Mode No AVX2). If both of these flags are passed, only the one that appears last is used.

Discussion (log): Luke Dashjr requested and introduced the topic, “Autotools forces user CXXFLAGS after our own, so when the user builds with -mno-avx2, the build simply fails.”

Wladimir van der Laan said, “It looks like a really contrived scenario to me—not worth it polluting the code with all kinds of compiler specific pragmas, at least.”

Cory Fields added, “I assume the issue is some failures to compile because of a busted compiler, so there’s a desire to be able to avoid them entirely.”

Conclusion: Dashjr argued that the issue was a must-fix for Bitcoin Core 0.17, but van der Laan, Marco Falke, and Gregory Maxwell disagreed. It seemed likely that additional information from the reporting user will be solicited to see why they need to pass -mno-avx2.

Windows issues with unicode in filenames

Background: as discussed last meeting, the interface built into Microsoft Windows prevents Bitcoin Core from easily opening files containing non-Latin characters under certain circumstances.

Discussion (log): Sjors Proovost requested and introduced the topic: “Do we want to fix the Windows unicode stuff given that there’s still two weeks [left for final development for Bitcoin Core 0.17]? I think the opinion in the ticket was no.”

Cory Fields noted that “due to the nature of the issue, I think many of the people who would be reporting it may not speak English, so the significance may be a little under represented.”

One particular difficulty with the issue is that there appears to be no way to reproduce it without access to a Windows system configured a particular way, so developers who don’t use Windows (the majority of active contributors) can’t directly work on it and Bitcoin Core’s automated tests can’t be used to prevent future regressions even if the bug is fixed.

Conclusion: Marco Falke noted that fixing the issue “would require a leveldb [upgrade] and major changes.” He suggested aiming to resolve the issue in 0.18 and several meeting participants seemed to agree. He further suggested that “we can backport it to 0.17.1, if it qualifies as a bug fix,” to which meeting participants clearly agreed.

LevelDB FD usage on x86_64

Background: Bitcoin Core uses the key-value store database (DB) LevelDB to track the set of Unspent Transaction Outputs (UTXOs)—all the spendable groups of bitcoins—as well as for an optional transaction index Bitcoin Core supports. LevelDB uses lots of relatively small files (~2 MB) to store its data. When it reads from those files, it prefers reading in a particular efficient way using Memory Mapping (mmap), but if that’s not possible, it falls back to directly reading from the disk drive using the select system call (syscall) with File Descriptors (FDs). However, the select syscall is severly limited in the number of FDs it can have open.

Discussion (log): Gregory Maxwell requested and introduced the topic: “There was a recent report from a user hitting the select limit on his x86_64 linux host. Inspection with lsof [list open files] shows that leveldb is using a lot of FDs on nodes where we expected it to be mostly using mmap. Apparently leveldb has a number of mmaps limit. As far as I know there isn’t any reason we shouldn’t increase it. Separately, we should move to using poll, but increasing the mmap limit should be a ~1 line change unless someone knows a reason to not do so.”

The mmap limit was discussed and none of the participants knew of a reason not to increase it. Just after the end of the meeting, Suhas Daftuar found an issue for leveldb that may indicate why the initial limit was placed there, which seemed to support increasing the limit on x86_64 systems.

Interwoven into the discussion of leveldb was the topic of switching Bitcoin Core from the older select syscall to the newer poll syscall for opening File Descriptors (FDs), which includes not just data files but also network ports. This low-level change would eliminate a problem where select can only handle a low maximum number of FDs and so Bitcoin Core is limited in various ways (for example, even if you increase the default maximum number of connections, it can’t handle many more connections). One problem with this change is that Windows does not implement an equivalent poll syscall, so some compatibility code would need to be written.

Conclusion: it seemed likely from the discussion that the LevelDB mmap limit would be increased from its current 1,000 to about 4,000 for the 0.17 Bitcoin Core release. It seemed unlikely that select would be changed to poll for that release as there’s not a satisfactory amount of time for testing, but nobody objected to changing it for a release in the subsequent planned major version (tentatively 0.18).

Note: discussion of this topic continued for about twenty minutes after the official end of the meeting.

Humor

Background: the IRC channel has been suffering from spam attacks recently, so the channel mode was set to quiet (+q) users without registered accounts.

        wumpus  topics?
       luke-jr  crickets
        wumpus  crickets are... good I guess
      gmaxwell  can someone please drop the registed users
                +q for now? sdaftuar is muted.
provoostenator  (I guess it was crickets and the muffled voice
                of sdaftuar in the distance)

Participants

IRC nick Name/Nym
wumpus Wladimir van der Laan
gmaxwell Gregory Maxwell
cfields Cory Fields
luke-jr Luke Dashjr
provoostenator Sjors Provoost
MarcoFalke Marco Falke
meshcollider Samuel Dobson
ken2812221 Chun Kuan Lee
sipa Pieter Wuille
jonasschnelli Jonas Schnelli
ossifrage Clem Taylor
sdaftuar Suhas Daftuar
instagibbs Gregory Sanders
jnewbery John Newbery
phantomcircuit Patrick Strateman
kanzure Bryan Bishop
midnightmagic Midnight Magic
promag Joao Barbosa
achow101 Andrew Chow

Disclaimer

This summary was compiled without input from any of the participants in the discussion, so any errors are the fault of the summary author and not the discussion participants. In particular, quotes taken from the discussion had their capitalization, punctuation, and spelling modified to produce consistent sentences. Bracketed words and fragments, as well as background narratives and exposition, were added by the author of this summary and may have accidentally changed the meaning of some sentences. If you believe any quote was taken out of context, please open an issue and we will correct the mistake.