Skip to content

Commit

Permalink
Use long instead of int for thread indices
Browse files Browse the repository at this point in the history
This protects against an integer overflow which could occur for large key list size and large thread counts.

Regrettably, it's difficult to write a regression test for this scenario, as exercising this overflow requires lots of time and heap, so it would be a performance regression to our test suites.

Fixes #19445

PiperOrigin-RevId: 595420516
Change-Id: Ic0a475a6a273c50fe9895dd0852fa5b062859cb2
  • Loading branch information
c-parsons authored and copybara-github committed Jan 3, 2024
1 parent eac7c96 commit 3e373d0
Showing 1 changed file with 8 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -274,18 +274,18 @@ protected void runInternal(ImmutableList<Pair<SkyKey, InvalidationType>> pending
MIN_TIME_FOR_LOGGING)) {
// To avoid contention and scheduling too many jobs for our #cpus, we start
// DEFAULT_THREAD_COUNT jobs, each processing a chunk of the pending visitations.
int listSize = pendingList.size();
int numThreads = min(DEFAULT_THREAD_COUNT, listSize);
for (int i = 0; i < numThreads; i++) {
int index = i;
long listSize = pendingList.size();
long numThreads = min(DEFAULT_THREAD_COUNT, listSize);
for (long i = 0; i < numThreads; i++) {
// Use long multiplication to avoid possible overflow, as numThreads * listSize might be
// larger than max int.
int startIndex = (int) ((i * listSize) / numThreads);
int endIndex = (int) (((i + 1) * listSize) / numThreads);
executor.execute(
() ->
visit(
Collections2.transform(
pendingList.subList(
(index * listSize) / numThreads,
((index + 1) * listSize) / numThreads),
Pair::getFirst),
pendingList.subList(startIndex, endIndex), Pair::getFirst),
InvalidationType.DELETED));
}
}
Expand Down

2 comments on commit 3e373d0

@fmeum
Copy link
Collaborator

@fmeum fmeum commented on 3e373d0 Jan 7, 2025

Choose a reason for hiding this comment

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

@iancha1992 Could we cherry-pick this into 7.5.0?

@iancha1992
Copy link
Member

@iancha1992 iancha1992 commented on 3e373d0 Jan 8, 2025

Choose a reason for hiding this comment

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

@fmeum Cherry-picked in #24866

Please sign in to comment.