Rubber Stamper (Gerrit)

unread,

Jan 13, 2026, 11:44:22 PMJan 13

to Ayumi Ono, Benoit Lize, Stephen Nusko, Chromium LUCI CQ, Eriko Kurimoto, Mikihito Matsuura, AyeAye, luci-bi...@appspot.gserviceaccount.com, Code Review Nudger, chromium...@chromium.org, chrom...@appspot.gserviceaccount.com, Kentaro Hara, kinuko...@chromium.org, lize...@chromium.org, wfh+...@chromium.org, bartek...@chromium.org, blink-...@chromium.org, tracing...@chromium.org, spang...@chromium.org, blink-re...@chromium.org, lizeb...@chromium.org

Attention needed from Benoit Lize, Eriko Kurimoto, Mikihito Matsuura and Stephen Nusko

Rubber Stamper voted Bot-Commit+1

Bot-Commit

+1

Open in Gerrit

Related details

Attention is currently required from:

Benoit Lize
Eriko Kurimoto
Mikihito Matsuura
Stephen Nusko

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Rubber Stamper (Gerrit)

unread,

Jan 13, 2026, 11:52:14 PMJan 13

to Ayumi Ono, Benoit Lize, Stephen Nusko, Chromium LUCI CQ, Eriko Kurimoto, Mikihito Matsuura, AyeAye, luci-bi...@appspot.gserviceaccount.com, Code Review Nudger, chromium...@chromium.org, chrom...@appspot.gserviceaccount.com, Kentaro Hara, kinuko...@chromium.org, lize...@chromium.org, wfh+...@chromium.org, bartek...@chromium.org, blink-...@chromium.org, tracing...@chromium.org, spang...@chromium.org, blink-re...@chromium.org, lizeb...@chromium.org

Attention needed from Benoit Lize, Eriko Kurimoto, Mikihito Matsuura and Stephen Nusko

Message from Rubber Stamper

Gerrit GetPureRevert API does not mark this CL as a pure revert. Learn more: go/rubber-stamper-user-guide.

Open in Gerrit

Related details

Attention is currently required from:

Benoit Lize
Eriko Kurimoto
Mikihito Matsuura
Stephen Nusko

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Mikihito Matsuura (Gerrit)

unread,

Jan 14, 2026, 12:24:26 AMJan 14

to Ayumi Ono, Benoit Lize, Stephen Nusko, Chromium LUCI CQ, Eriko Kurimoto, AyeAye, luci-bi...@appspot.gserviceaccount.com, Code Review Nudger, chromium...@chromium.org, chrom...@appspot.gserviceaccount.com, Kentaro Hara, kinuko...@chromium.org, lize...@chromium.org, wfh+...@chromium.org, bartek...@chromium.org, blink-...@chromium.org, tracing...@chromium.org, spang...@chromium.org, blink-re...@chromium.org, lizeb...@chromium.org

Attention needed from Ayumi Ono, Benoit Lize, Eriko Kurimoto and Stephen Nusko

Mikihito Matsuura voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Ayumi Ono
Benoit Lize
Eriko Kurimoto
Stephen Nusko

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Stephen Nusko (Gerrit)

unread,

Jan 14, 2026, 12:29:53 AMJan 14

to Ayumi Ono, Mikihito Matsuura, Benoit Lize, Chromium LUCI CQ, Eriko Kurimoto, AyeAye, luci-bi...@appspot.gserviceaccount.com, Code Review Nudger, chromium...@chromium.org, chrom...@appspot.gserviceaccount.com, Kentaro Hara, kinuko...@chromium.org, lize...@chromium.org, wfh+...@chromium.org, bartek...@chromium.org, blink-...@chromium.org, tracing...@chromium.org, spang...@chromium.org, blink-re...@chromium.org, lizeb...@chromium.org

Attention needed from Ayumi Ono, Benoit Lize and Eriko Kurimoto

Stephen Nusko voted and added 3 comments

Votes added by Stephen Nusko

Code-Review

+1

3 comments

Commit Message

Line 11, Patchset 4 (Latest):Reason for revert: fixed

Stephen Nusko . unresolved

Can you expand on this? What did you change? Which patchset to diff to see the changes from the original?

File base/allocator/partition_allocator/src/partition_alloc/thread_cache_unittest.cc

Line 93, Patchset 4 (Latest): opts.thread_cache_index = internal::kDefaultRootThreadCacheIndex;

Stephen Nusko . unresolved

I think this is the change?

File third_party/blink/renderer/platform/wtf/allocator/partitions.cc

Line 147, Patchset 4 (Latest): options.thread_cache_index = 0;

Stephen Nusko . unresolved

This is just a magic number, could you either

1) Add a comment explaining why zero
2) Add a descriptively named variable set to zero and use that?

---------------------------

Optional further thought:

Also this is sort of hard for future users to know how to pick this value, obviously here they don't have PA-E, and the point is this is all known at compile time but perhaps we should have a `partition_alloc::GetNextAvailableThreadCacheIndex(partiton_alloc::ThreadCache::FastMalloc)`?

And then we explicitly enumerate all allowed usages and pass the correct values? This might make it be very clear what you need to do to add a new PartitionRoot with a thread cache.

Again this second half is completely optional, for this CL feel free to just add a comment about the zero or a named variable.

Open in Gerrit

Related details

Attention is currently required from:

Ayumi Ono
Benoit Lize
Eriko Kurimoto

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Ayumi Ono (Gerrit)

unread,

Jan 14, 2026, 12:53:04 AMJan 14

to Stephen Nusko, Mikihito Matsuura, Benoit Lize, Chromium LUCI CQ, Eriko Kurimoto, AyeAye, luci-bi...@appspot.gserviceaccount.com, Code Review Nudger, chromium...@chromium.org, chrom...@appspot.gserviceaccount.com, Kentaro Hara, kinuko...@chromium.org, lize...@chromium.org, wfh+...@chromium.org, bartek...@chromium.org, blink-...@chromium.org, tracing...@chromium.org, spang...@chromium.org, blink-re...@chromium.org, lizeb...@chromium.org

Attention needed from Benoit Lize and Eriko Kurimoto

Ayumi Ono voted and added 3 comments

Votes added by Ayumi Ono

Commit-Queue

+1

3 comments

Commit Message

Line 11, Patchset 4:Reason for revert: fixed

Stephen Nusko . resolved

Can you expand on this? What did you change? Which patchset to diff to see the changes from the original?

Ayumi Ono

Done

File base/allocator/partition_allocator/src/partition_alloc/thread_cache_unittest.cc

Line 93, Patchset 4: opts.thread_cache_index = internal::kDefaultRootThreadCacheIndex;

Stephen Nusko . resolved

I think this is the change?

Ayumi Ono

Yes

File third_party/blink/renderer/platform/wtf/allocator/partitions.cc

Line 147, Patchset 4: options.thread_cache_index = 0;

Stephen Nusko . resolved

This is just a magic number, could you either

1) Add a comment explaining why zero
2) Add a descriptively named variable set to zero and use that?
---------------------------
Optional further thought:
Also this is sort of hard for future users to know how to pick this value, obviously here they don't have PA-E, and the point is this is all known at compile time but perhaps we should have a `partition_alloc::GetNextAvailableThreadCacheIndex(partiton_alloc::ThreadCache::FastMalloc)`?
And then we explicitly enumerate all allowed usages and pass the correct values? This might make it be very clear what you need to do to add a new PartitionRoot with a thread cache.
Again this second half is completely optional, for this CL feel free to just add a comment about the zero or a named variable.

Ayumi Ono

I fixed to use `partition_alloc::internal::kDefaultRootThreadCacheIndex`.
About the second half, it makes sense, but I think it's not feasible because the thread index occupation is different per threads and time; the default partition's thread_cache is currently configured during initialization, and not always configured.

Open in Gerrit

Related details

Attention is currently required from:

Benoit Lize
Eriko Kurimoto

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Ayumi Ono (Gerrit)

unread,

Jan 14, 2026, 1:53:48 AMJan 14

to Takashi Sakamoto, Stephen Nusko, Mikihito Matsuura, Chromium LUCI CQ, Eriko Kurimoto, AyeAye, luci-bi...@appspot.gserviceaccount.com, Code Review Nudger, chromium...@chromium.org, chrom...@appspot.gserviceaccount.com, Kentaro Hara, kinuko...@chromium.org, lize...@chromium.org, wfh+...@chromium.org, bartek...@chromium.org, blink-...@chromium.org, tracing...@chromium.org, spang...@chromium.org, blink-re...@chromium.org, lizeb...@chromium.org

Attention needed from Eriko Kurimoto and Takashi Sakamoto

Ayumi Ono voted and added 1 comment

Votes added by Ayumi Ono

Commit-Queue

+1

1 comment

Patchset-level comments

File-level comment, Patchset 8 (Latest):

Ayumi Ono . resolved

PTAL
Patch 1 was already approved in "PA: Support multiple ThreadCache instances per thread". Patch 2 and 5 add new changes.

Open in Gerrit

Related details

Attention is currently required from:

Eriko Kurimoto
Takashi Sakamoto

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Takashi Sakamoto (Gerrit)

unread,

Jan 14, 2026, 1:55:06 AMJan 14

to Ayumi Ono, Stephen Nusko, Mikihito Matsuura, Chromium LUCI CQ, Eriko Kurimoto, AyeAye, luci-bi...@appspot.gserviceaccount.com, Code Review Nudger, chromium...@chromium.org, chrom...@appspot.gserviceaccount.com, Kentaro Hara, kinuko...@chromium.org, lize...@chromium.org, wfh+...@chromium.org, bartek...@chromium.org, blink-...@chromium.org, tracing...@chromium.org, spang...@chromium.org, blink-re...@chromium.org, lizeb...@chromium.org

Attention needed from Ayumi Ono and Eriko Kurimoto

Takashi Sakamoto voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Ayumi Ono
Eriko Kurimoto

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Ayumi Ono (Gerrit)

unread,

Jan 14, 2026, 5:13:14 AMJan 14

to Takashi Sakamoto, Stephen Nusko, Mikihito Matsuura, Chromium LUCI CQ, Eriko Kurimoto, AyeAye, luci-bi...@appspot.gserviceaccount.com, Code Review Nudger, chromium...@chromium.org, chrom...@appspot.gserviceaccount.com, Kentaro Hara, kinuko...@chromium.org, lize...@chromium.org, wfh+...@chromium.org, bartek...@chromium.org, blink-...@chromium.org, tracing...@chromium.org, spang...@chromium.org, blink-re...@chromium.org, lizeb...@chromium.org

Attention needed from Eriko Kurimoto

Ayumi Ono voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention is currently required from:

Eriko Kurimoto

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

Jan 14, 2026, 5:32:53 AMJan 14

to Ayumi Ono, Takashi Sakamoto, Stephen Nusko, Mikihito Matsuura, Eriko Kurimoto, AyeAye, luci-bi...@appspot.gserviceaccount.com, Code Review Nudger, chromium...@chromium.org, chrom...@appspot.gserviceaccount.com, Kentaro Hara, kinuko...@chromium.org, lize...@chromium.org, wfh+...@chromium.org, bartek...@chromium.org, blink-...@chromium.org, tracing...@chromium.org, spang...@chromium.org, blink-re...@chromium.org, lizeb...@chromium.org

Chromium LUCI CQ submitted the change

Change information

Commit message:

Reland "PA: Support multiple ThreadCache instances per thread."

This reverts commit 101ec6b775b79e6a644521d9213b3225237a1ee1.

Reason for revert: fixed by patchset 2, which adds the `opts.thread_cache_index =
internal::kDefaultRootThreadCacheIndex` when creating an allocator for
thread cache tests.
The previous failure example is https://ci.chromium.org/b/8692743788900425585.

Confirmed by running fuchsia-x64-cast-receiver-dbg,linux-chromeos-dbg,win10-dbg,android-rust-arm64-dbg, which failed in the reverted CL.

Original change's description:
> Revert "PA: Support multiple ThreadCache instances per thread."
>
> This reverts commit d7bfc3c111e238f13ccc81110cb8550cccb5a6a3.
>
> Reason for revert: Culprit of the wide-spread breakage of
> AlternateBucketDistributionAndPartitionFreeList*
>
> Example failure log:
> https://ci.chromium.org/ui/p/chromium/builders/ci/linux-chromeos-dbg/43949/overview
>
> Original change's description:


> > PA: Support multiple ThreadCache instances per thread.
> >

> > Currently, each thread only supports a single ThreadCache, which is
> > exclusively used by the default partition. This limitation hinders
> > security improvements that involve splitting the default partition into
> > several smaller partitions, as it leads to performance regressions due
> > to the lack of a thread cache for these new partitions.
> >
> > The design document is available in
> > https://docs.google.com/document/d/17wkGZ3ekQoirb0--kls-Sblr5qRSjtZ0VjtuJ5WIPH8/edit?usp=sharing
> > (internal only).
> >
> > This change introduces support for multiple `ThreadCache` instances per
> > thread. The number of thread caches is configured at compile time. These
> > caches are managed using:
> > -   An array of `ThreadCache*` in `thread_local`.
> > -   An array of `ThreadCache` within `PartitionTLS`.
> >
> > For `PartitionTLS`, an additional check ensures each `ThreadCache` is
> > properly initialized, as thread caches from different roots are created
> > at different times. The `IsTombstone` function has also been modified to
> > check against the entire thread's state rather than requiring a specific
> > `ThreadCache` pointer, since all thread caches for a given thread are
> > destroyed simultaneously.
> >
> > Note that additional testing, especially in `thread_cache_unittest.cc`,
> > will be added in a subsequent CL.
> >
> > Bug: 467243745
> > Change-Id: I217215f6db648ffba2f7313af2bdb546e36f6fe1
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7206958
> > Auto-Submit: Ayumi Ono <ayum...@google.com>
> > Reviewed-by: Benoit Lize <li...@chromium.org>
> > Commit-Queue: Ayumi Ono <ayum...@google.com>
> > Cr-Commit-Position: refs/heads/main@{#1568807}
>
> Bug: 467243745
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Change-Id: I8263c8be7b03694fefa6985609a230b62fc481a5
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7458168
> Commit-Queue: Rubber Stamper <rubber-...@appspot.gserviceaccount.com>
> Auto-Submit: Eriko Kurimoto <elk...@chromium.org>
> Owners-Override: Eriko Kurimoto <elk...@chromium.org>
> Bot-Commit: Rubber Stamper <rubber-...@appspot.gserviceaccount.com>
> Cr-Commit-Position: refs/heads/main@{#1568894}

Cq-Include-Trybots: luci.chromium.try:fuchsia-x64-cast-receiver-dbg,linux-chromeos-dbg,win10-dbg,android-rust-arm64-dbg

Bug: 467243745

Change-Id: Idf77da44e59566ef83bb7a412f15b45f8ea43b8f

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7462014

Reviewed-by: Stephen Nusko <nus...@chromium.org>

Reviewed-by: Mikihito Matsuura <mi...@google.com>

Reviewed-by: Takashi Sakamoto <ta...@google.com>

Commit-Queue: Ayumi Ono <ayum...@google.com>

Cr-Commit-Position: refs/heads/main@{#1569000}

Files:

M base/allocator/partition_allocator/src/partition_alloc/extended_api.cc
M base/allocator/partition_allocator/src/partition_alloc/memory_reclaimer_unittest.cc
M base/allocator/partition_allocator/src/partition_alloc/partition_alloc.cc
M base/allocator/partition_allocator/src/partition_alloc/partition_alloc_unittest.cc
M base/allocator/partition_allocator/src/partition_alloc/partition_root.cc
M base/allocator/partition_allocator/src/partition_alloc/partition_root.h
M base/allocator/partition_allocator/src/partition_alloc/scheduler_loop_quarantine.cc
M base/allocator/partition_allocator/src/partition_alloc/scheduler_loop_quarantine_support.cc
M base/allocator/partition_allocator/src/partition_alloc/scheduler_loop_quarantine_support.h
M base/allocator/partition_allocator/src/partition_alloc/scheduler_loop_quarantine_unittest.cc
M base/allocator/partition_allocator/src/partition_alloc/thread_cache.cc
M base/allocator/partition_allocator/src/partition_alloc/thread_cache.h
M base/allocator/partition_allocator/src/partition_alloc/thread_cache_unittest.cc
M base/trace_event/memory_infra_background_allowlist.cc
M third_party/blink/renderer/platform/wtf/allocator/partitions.cc

Change size: L

Delta: 15 files changed, 324 insertions(+), 139 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Stephen Nusko, +1 by Takashi Sakamoto, +1 by Mikihito Matsuura

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Takashi Sakamoto (Gerrit)

unread,

Feb 25, 2026, 9:25:20 PM (2 days ago) Feb 25

to Chromium LUCI CQ, Ayumi Ono, Stephen Nusko, Mikihito Matsuura, Eriko Kurimoto, AyeAye, luci-bi...@appspot.gserviceaccount.com, Code Review Nudger, chromium...@chromium.org, chrom...@appspot.gserviceaccount.com, Kentaro Hara, kinuko...@chromium.org, lize...@chromium.org, wfh+...@chromium.org, bartek...@chromium.org, blink-...@chromium.org, tracing...@chromium.org, spang...@chromium.org, blink-re...@chromium.org, lizeb...@chromium.org

Attention needed from Ayumi Ono

Takashi Sakamoto added 1 comment

File base/allocator/partition_allocator/src/partition_alloc/thread_cache.cc

Line 138, Patchset 9 (Latest): continue;

Takashi Sakamoto . unresolved

... this looks a bug. If `tcache != nullptr` and `tcache->root_->settings.thread_cache_index != index`, the code does `continue` without updating `tcache`. The `while` will never terminate, I think.

Open in Gerrit

Related details

Attention is currently required from:

Ayumi Ono

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Stephen Nusko (Gerrit)

unread,

Feb 26, 2026, 2:37:33 AM (2 days ago) Feb 26

to Chromium LUCI CQ, Ayumi Ono, Takashi Sakamoto, Mikihito Matsuura, Eriko Kurimoto, AyeAye, luci-bi...@appspot.gserviceaccount.com, Code Review Nudger, chromium...@chromium.org, chrom...@appspot.gserviceaccount.com, Kentaro Hara, kinuko...@chromium.org, lize...@chromium.org, wfh+...@chromium.org, bartek...@chromium.org, blink-...@chromium.org, tracing...@chromium.org, spang...@chromium.org, blink-re...@chromium.org, lizeb...@chromium.org

Attention needed from Ayumi Ono

Stephen Nusko added 1 comment

File base/allocator/partition_allocator/src/partition_alloc/thread_cache.cc

Line 138, Patchset 9 (Latest): continue;

Takashi Sakamoto . unresolved

... this looks a bug. If `tcache != nullptr` and `tcache->root_->settings.thread_cache_index != index`, the code does `continue` without updating `tcache`. The `while` will never terminate, I think.

Stephen Nusko

Yeah there is also a bug in [scheduler_loop_quarantine_support.cc](https://source.chromium.org/chromium/chromium/src/+/main:base/allocator/partition_allocator/src/partition_alloc/scheduler_loop_quarantine_support.cc;l=15;drc=150979b30b232291e2efc8de7114caadda909920) where if a single TCache is is invalid we do a `return` and skip the rest of the loop when it should be a `continue`. I feel like we might want to do a audit of thread cache looping (I.E. calls to ThreadCache::Get() and list_head_)

Can you file a bug? so we do it systematically for all callsites.

satisfied_requirement

open

diffy

Reply all

Reply to author

Forward

Reland "PA: Support multiple ThreadCache instances per thread." [chromium/src : main]

Rubber Stamper (Gerrit)

Rubber Stamper voted Bot-Commit+1

Related details

Rubber Stamper (Gerrit)

Message from Rubber Stamper

Related details

Mikihito Matsuura (Gerrit)

Mikihito Matsuura voted Code-Review+1

Related details

Stephen Nusko (Gerrit)

Stephen Nusko voted and added 3 comments

Votes added by Stephen Nusko

3 comments

Related details

Ayumi Ono (Gerrit)

Ayumi Ono voted and added 3 comments

Votes added by Ayumi Ono

3 comments

Related details

Ayumi Ono (Gerrit)

Ayumi Ono voted and added 1 comment

Votes added by Ayumi Ono

1 comment

Related details

Takashi Sakamoto (Gerrit)

Takashi Sakamoto voted Code-Review+1

Related details

Ayumi Ono (Gerrit)

Ayumi Ono voted Commit-Queue+2

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Change information

Takashi Sakamoto (Gerrit)

Takashi Sakamoto added 1 comment

Related details

Stephen Nusko (Gerrit)

Stephen Nusko added 1 comment