IRC meeting summary for 2018-08-02
Overview
- View this week’s log on BotBot.me or MeetBot
- Meeting minutes by MeetBot
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.
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.