Skip to content

DISPATCH-1295 - Reduce lock contention and make fewer system calls.#476

Closed
cliffjansen wants to merge 1 commit intoapache:masterfrom
cliffjansen:d1295
Closed

DISPATCH-1295 - Reduce lock contention and make fewer system calls.#476
cliffjansen wants to merge 1 commit intoapache:masterfrom
cliffjansen:d1295

Conversation

@cliffjansen
Copy link

Flamegraphs show on a single quiver run that there is lock contention in setting timers. Locks are held across system calls and also cause thread stalls as seen in offcpu runs.

This change restricts the main timer lock to updating internal data structures. A second lock is used to prevent setting timer values out of order.

The main benefit is that immediate timer calls have much less contention with non-immediate timers.

Using a standard quiver C client run of 9,000,000 message, which lasts about a minute, we get 19% of the offcpu related to qd_timer_schedule and qd_timer_visit. This drops to 0 with the change.

Regular on-cpu profiling samples in qd_timer drop from 9.1% of samples to 0.2%. Samples in qd_immediate drop modestly from 0.6% to 0.5%.

@codecov-io
Copy link

Codecov Report

Merging #476 into master will increase coverage by 0.12%.
The diff coverage is 89.83%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #476      +/-   ##
==========================================
+ Coverage   86.97%   87.09%   +0.12%     
==========================================
  Files          86       86              
  Lines       19281    19332      +51     
==========================================
+ Hits        16770    16838      +68     
+ Misses       2511     2494      -17
Impacted Files Coverage Δ
src/immediate.c 81.15% <84.84%> (-16.28%) ⬇️
src/timer.c 94.4% <96.15%> (+0.16%) ⬆️
src/parse.c 87.9% <0%> (-0.26%) ⬇️
src/container.c 79.77% <0%> (+0.18%) ⬆️
src/router_core/router_core.c 87.09% <0%> (+0.21%) ⬆️
src/router_pynode.c 87.96% <0%> (+3.7%) ⬆️
src/router_core/route_tables.c 81.88% <0%> (+5.21%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 106ba6c...7765a6a. Read the comment docs.

@cliffjansen
Copy link
Author

Since this pull request builds on top of DISPATCH-1274, and that has been removed for the next release, I am closing this for now. Next steps TBD.

ChugR pushed a commit to ChugR/qpid-dispatch that referenced this pull request Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants