Skip to content

chore: remove mt.h (last users migrated to ROOT::EnableImplicitMT())#4

Merged
wdconinc merged 1 commit intomainfrom
remove-mt-header
Apr 28, 2026
Merged

chore: remove mt.h (last users migrated to ROOT::EnableImplicitMT())#4
wdconinc merged 1 commit intomainfrom
remove-mt-header

Conversation

@wdconinc
Copy link
Copy Markdown
Contributor

@wdconinc wdconinc commented Apr 27, 2026

Summary

include/common_bench/mt.h is now unused. The three scripts in physics_benchmarks that previously called ROOT::EnableImplicitMT(kNumThreads) have been updated to ROOT::EnableImplicitMT() (see companion PR eic/physics_benchmarks#95).

Why this is correct

mt.h defined constexpr int kNumThreads = 8 — a hardcoded value that:

  • Ignored $BENCHMARK_N_THREADS from env.sh (CI default: 10)
  • Was inconsistent with all other analysis scripts, which already used the no-arg form
  • Carried a TODO comment noting it should be made configurable (never done)

Changes

  • Delete include/common_bench/mt.h (11 lines)

Related

Part of an ongoing audit to streamline common_bench usage:

Copilot AI review requested due to automatic review settings April 27, 2026 20:33
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Removes the now-unused include/common_bench/mt.h header that hardcoded a fixed thread count, aligning common_bench with the repo’s broader move to ROOT::EnableImplicitMT() without an explicit thread number.

Changes:

  • Delete include/common_bench/mt.h (hardcoded kNumThreads = 8).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

mt.h defined a single compile-time constant (kNumThreads = 8) intended
to control ROOT's implicit multithreading. It was ignored by the
majority of analysis scripts, which already used ROOT::EnableImplicitMT()
with no argument. The three remaining users in physics_benchmarks have
now been updated to use the no-arg form.

The header's own TODO noted it should be made configurable via CI
scripts — that is now moot since auto-detection is the correct approach.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@wdconinc wdconinc enabled auto-merge (squash) April 28, 2026 19:19
@wdconinc wdconinc merged commit de3f97b into main Apr 28, 2026
4 checks passed
@wdconinc wdconinc deleted the remove-mt-header branch April 28, 2026 23:57
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