Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the number of "camera scans" constant in the philosophers benchmark #447

Merged
merged 5 commits into from
Jul 17, 2024

Conversation

lbulej
Copy link
Member

@lbulej lbulej commented Jun 1, 2024

This includes two related changes and one extra bit.

The first reduces the duration of the transaction building the "image of the scene". Previously the image was being built inside the transaction, making the transaction last unnecessarily longer. Now we only take a snapshot of the state within the transaction and build the image outside the transaction to avoid blocking philosopher threads.

The second change makes the number of scans constant for a given workload. Previously, camera scans were triggered in reference to wall clock time, resulting in variable number of scans during each repetition. Now the camera scans are requested based on thread progress. To keep the number of camera scans constant, the requests are counted to avoid potential loss of wake-ups when the rate of progress increases (due to reduced contention towards the end of the workload).

These two changes should alleviate the banding in the operation durations (#202), but probably not entirely, because there simply is a thread that periodically blocks all threads for a while.

The extra bit is inclusion of the number of camera scans in the validation.

Below are plots from simple measurements I collected from 15 runs (150 repetitions each) using JDK 11 (default settings) on 8 cores of Xeon E5-2697A v4:

  • the split_scan variant corresponds to commit d3a614a
  • the const_scan_lbq variant corresponds to commit 18b4936 (scan requests queued using LinkedBlockingQueue)
  • the const_scan_stm variant corresponds to commit 05ad765 (scan requests "queued" using STM transactions)

plot-runs-samples
plot-runs-histograms
plot-runs-violins

@lbulej lbulej marked this pull request as ready for review June 2, 2024 11:04
Reduces visibility of some elements, uses named fork objects, moves
status formatting to the "image" capturing method, and adds white
space around separate logical steps.
Take a snapshot of the state within a transaction and then format it
(outside the transaction) to avoid blocking the mutators. This is what
actual state monitoring code should be doing.
Camera scans are requested based on thread progress. To keep the number
of camera scans constant, the requests are queued to avoid potential
loss of wake-ups when the rate of progress increases (due to reduced
contention towards the end of the workload).

This is a proof-of-concept, it would be nice to replace the
LinkedBlockingQueue with an STM-based solution.
Uses simple counters instead to ensure that we always perform a fixed
number of scans for a given workload and that the camera thread
terminates when the number of scans reached the expected total.
Also adds a `block_meal_count` parameter which determines the number
of meals that represents a block of progress. The size of the block
determines the number of camera scans, which the benchmark validates.
@lbulej lbulej force-pushed the topic/philosophers-const-scan branch from 169cbe3 to 05ad765 Compare July 17, 2024 10:01
@lbulej
Copy link
Member Author

lbulej commented Jul 17, 2024

Rebased to master and updated the commit ids in the description.

@lbulej lbulej requested a review from axel22 July 17, 2024 10:10
Copy link
Member

@axel22 axel22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@lbulej lbulej merged commit 6267add into master Jul 17, 2024
13 checks passed
@lbulej lbulej deleted the topic/philosophers-const-scan branch July 17, 2024 11:36
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