Page MenuHomePhabricator

Merge #12618: Set SCHED_BATCH priority on the loadblk thread.
ClosedPublic

Authored by nakihito on Aug 28 2019, 00:50.

Details

Summary

d54874d Set SCHED_BATCH priority on the loadblk thread. (Evan Klitzke)

Pull request description:

Today I came across #10271, and while reading the discussion #6358 was linked to. Linux systems have a `SCHED_BATCH` scheduler priority that is useful for threads like loadblk. You can find the full details at [sched(7)](http://man7.org/linux/man-pages/man7/sched.7.html), but I'll quote the relevant part of the man page below:

> ...this policy will cause the scheduler to always assume that the thread is
CPU-intensive. Consequently, the scheduler will apply a small scheduling penalty
with respect to wakeup behavior, so that this thread is mildly disfavored in
scheduling decisions.
>
> This policy is useful for workloads that are noninteractive, but do not want to
lower their nice value, and for workloads that want a deterministic scheduling
policy without interactivity causing extra preemptions (between the workload's
tasks).

I think this change is useful independently of #10271 and irrespective of whether that change is merged. Under normal operation the loadblk thread will just import `mempool.dat`. However, if Bitcoin is started with `-reindex` or `-reindex-chainstate` this thread will use a great deal of CPU while it rebuilds the chainstate database (and the block database in the case of `-reindex`). By setting `SCHED_BATCH` this thread is less likely to interfere with interactive tasks (e.g. the user's web browser, text editor, etc.).

I'm leaving the nice value unchanged (which also affects scheduling decisions) because I think that's better set by the user. Likewise I'm not using [ioprio_set(2)](http://man7.org/linux/man-pages/man2/ioprio_set.2.html) because it can cause the thread to become completely I/O starved (and knowledgeable users can use `ionice(1)` anyway).

Tree-SHA512: ea8f7d3921ed5708948809da771345cdc33efd7ba3323e9dfec07a25bc21e8612e2676f9c178e2710c7bc437e8c9cafc5e0463613688fea5699b6e8e2fec6cff

Backport of Core PR12618
https://github.com/bitcoin/bitcoin/pull/12618/

Test Plan
make check
test_runner.py

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

nakihito created this revision.Aug 28 2019, 00:50
Owners added a reviewer: Restricted Owners Package.Aug 28 2019, 00:50
Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 28 2019, 00:50

https://github.com/bitcoin/bitcoin/issues/12915
This is supposed to introduce the above bug, but I could not build with the same configure options to test. The fix is made in D3954.

deadalnix requested changes to this revision.Aug 28 2019, 14:13
deadalnix added inline comments.
src/util.h
374 ↗(On Diff #10976)
This revision now requires changes to proceed.Aug 28 2019, 14:13
nakihito updated this revision to Diff 11005.Aug 28 2019, 18:35

Fixed function(void) -> function().

nakihito updated this revision to Diff 11007.Aug 28 2019, 19:12

function(void) -> function().

nakihito updated this revision to Diff 11008.EditedAug 28 2019, 19:13

util.cpp change was accidentally skipped.

deadalnix accepted this revision.Aug 29 2019, 16:32
This revision is now accepted and ready to land.Aug 29 2019, 16:32