Skip to content

chore: add CPU pinning - #157

Merged
rluvaton merged 1 commit into
mainfrom
add-cpu-pinning
Apr 8, 2024
Merged

rluvaton merged 1 commit into
mainfrom
add-cpu-pinning

Conversation

@rluvaton

@rluvaton rluvaton commented Apr 8, 2024

Copy link
Copy Markdown
Member

@rluvaton
rluvaton requested a review from mcollina April 8, 2024 10:04

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@rluvaton
rluvaton merged commit c4f3d4e into main Apr 8, 2024
@rluvaton
rluvaton deleted the add-cpu-pinning branch April 8, 2024 10:58
echo "Output will be saved to $fileName"
pwd
./node-master benchmark/compare.js --old ./node-master --new ./node-pr $FILTER $RUNS -- $CATEGORY | tee $fileName
./node-master benchmark/compare.js --set CPUSET=0 --old ./node-master --new ./node-pr $FILTER $RUNS -- $CATEGORY | tee $fileName

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure CPUSET=0 is the desired setting -- wouldn't this set the benchmarks to only run on CPU 0?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nodejs/node#52233 (comment) suggests

So it should just be a matter of taskset -c 0-11 <command> for the perf cores.

So maybe CPUSET=0-11? Or perhaps a better approach would be to parameterize (similar to e.g. $FILTER, $RUNS, etc.) and then we can set the parameter in Jenkins.

rluvaton added a commit that referenced this pull request Apr 8, 2024
rluvaton added a commit that referenced this pull request Apr 8, 2024
* Run on CPUs 0-11 (perf cores)

Ref: #157 (comment)

* add option to run on specific cpu set
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.

3 participants