Mikihito Matsuura (Gerrit)

unread,

Dec 24, 2025, 1:37:49 AM12/24/25

to Keishi Hattori, Chromium LUCI CQ, chromium...@chromium.org, Kentaro Hara, bartek...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, lize...@chromium.org, lizeb...@chromium.org, wfh+...@chromium.org

Attention needed from Keishi Hattori

Mikihito Matsuura added 1 comment

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Mikihito Matsuura . resolved

PTAL

Open in Gerrit

Related details

Attention is currently required from:

Keishi Hattori

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,

Dec 25, 2025, 4:56:11 AM12/25/25

to Kalvin Lee, Keishi Hattori, Chromium LUCI CQ, chromium...@chromium.org, Kentaro Hara, bartek...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, lize...@chromium.org, lizeb...@chromium.org, wfh+...@chromium.org

Attention needed from Kalvin Lee and Keishi Hattori

Mikihito Matsuura voted and added 1 comment

Votes added by Mikihito Matsuura

Owners-Override

+1

1 comment

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Mikihito Matsuura . resolved

Kalvin, can you take a look? (Both Keishi-san and Sakamoto-san are OOO)

Open in Gerrit

Related details

Attention is currently required from:

Kalvin Lee
Keishi Hattori

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

Kalvin Lee (Gerrit)

unread,

Jan 5, 2026, 12:47:08 AM (4 days ago) Jan 5

to Mikihito Matsuura, Keishi Hattori, Chromium LUCI CQ, chromium...@chromium.org, Kentaro Hara, bartek...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, lize...@chromium.org, lizeb...@chromium.org, wfh+...@chromium.org

Attention needed from Keishi Hattori and Mikihito Matsuura

Kalvin Lee voted and added 2 comments

Votes added by Kalvin Lee

Code-Review

+1

2 comments

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Kalvin Lee . resolved

Sorry about the long long delay. One non-blocking nit, but feel free to overrule.

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

Line 131, Patchset 4 (Latest): constexpr size_t kLargeEmptySlotSpanRingSize = 1 << 7;

Kalvin Lee . resolved

Nit: this magic constant appears a fair bit in this CL. This is personal preference, but I might do something like

```
enum SlotSpanRingMaxSize : int16_t {
  kSmaller = 1 << 7,
  kLarger = 1 << 10,
};

inline constexpr size_t kMaxEmptySlotSpanRingSize =
#if PA_BUILDFLAG(USE_LARGE_EMPTY_SLOT_SPAN_RING)
  SlotSpanRingMaxSize::kLarger;
#else
  SlotSpanRingMaxSize::kSmaller;
#endif

// and in PA unit test, Blink, and `partition_alloc_features`,
// we can utter `SlotSpanRingMaxSize::kSmaller`.
```

and then we don't get the naked `1 << 7` traveling around the codebase.

I think most files here are allowed to include `partition_alloc_constants.h`, so hopefully we don't get any build / dependency complications out of this.

If I have misunderstood the constant (or you anticipate this will be a relatively short-lived change), feel free to ignore this comment 😊

Open in Gerrit

Related details

Attention is currently required from:

Keishi Hattori
Mikihito Matsuura

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

Mikihito Matsuura (Gerrit)

unread,

Jan 5, 2026, 2:00:46 AM (4 days ago) Jan 5

to Kalvin Lee, Keishi Hattori, Chromium LUCI CQ, chromium...@chromium.org, Kentaro Hara, bartek...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, lize...@chromium.org, lizeb...@chromium.org, wfh+...@chromium.org

Attention needed from Keishi Hattori

Mikihito Matsuura voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention is currently required from:

Keishi Hattori

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

Chromium LUCI CQ (Gerrit)

unread,

Jan 5, 2026, 4:19:10 AM (4 days ago) Jan 5

to Mikihito Matsuura, Kalvin Lee, Keishi Hattori, chromium...@chromium.org, Kentaro Hara, bartek...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, lize...@chromium.org, lizeb...@chromium.org, wfh+...@chromium.org

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

4 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: base/allocator/partition_alloc_features.cc
Insertions: 8, Deletions: 12.

The diff is too large to show. Please review the diff.
```
```
The name of the file: base/allocator/partition_allocator/src/partition_alloc/partition_alloc_constants.h
Insertions: 11, Deletions: 3.

The diff is too large to show. Please review the diff.
```
```
The name of the file: base/allocator/partition_allocator/src/partition_alloc/partition_alloc_unittest.cc
Insertions: 3, Deletions: 3.

The diff is too large to show. Please review the diff.
```
```
The name of the file: third_party/blink/renderer/platform/wtf/allocator/partitions.cc
Insertions: 2, Deletions: 1.

The diff is too large to show. Please review the diff.
```

Change information

Commit message:

[PA] Parametrize Empty Slot Span Ring Buffer size

This helps to fine-tune the constant through an experiment.
Ring size configuration is now handled outside PartitionAlloc, because
concept of "foreground" / "background" is Chrome-specific and should be
taken care of at `//base` layer.

Bug: 329199197

Change-Id: I4fadbd3fc0cae83a03e1fb5a1b2a97ba389de4af

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

Owners-Override: Mikihito Matsuura <mi...@google.com>

Reviewed-by: Kalvin Lee <kd...@chromium.org>

Commit-Queue: Mikihito Matsuura <mi...@google.com>

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

Files:

M base/allocator/partition_alloc_features.cc
M base/allocator/partition_alloc_features.h
M base/allocator/partition_alloc_support.cc
M base/allocator/partition_allocator/src/partition_alloc/partition_alloc_constants.h
M base/allocator/partition_allocator/src/partition_alloc/partition_alloc_unittest.cc
M base/allocator/partition_allocator/src/partition_alloc/partition_root.h
M base/allocator/partition_allocator/src/partition_alloc/shim/allocator_shim.h
M base/allocator/partition_allocator/src/partition_alloc/shim/allocator_shim_default_dispatch_to_partition_alloc.cc
M third_party/blink/renderer/platform/wtf/allocator/partitions.cc

Change size: M

Delta: 9 files changed, 127 insertions(+), 98 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Kalvin Lee

Open in Gerrit

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

Gerrit

open

diffy

satisfied_requirement

Michael Lippautz (Gerrit)

unread,

Jan 7, 2026, 8:39:17 AM (2 days ago) Jan 7

to Chromium LUCI CQ, Mikihito Matsuura, Kalvin Lee, Keishi Hattori, chromium...@chromium.org, Kentaro Hara, bartek...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, lize...@chromium.org, lizeb...@chromium.org, wfh+...@chromium.org

Michael Lippautz added 1 comment

Patchset-level comments

File-level comment, Patchset 7 (Latest):

Michael Lippautz . resolved

Are you now parameterizing the value and working on the linked bug now? We are using the older foreground notify function (in the shim) in d8 and need d8 to be aligned with what's going on here to get meaningful profiles.

Open in Gerrit

Related details

Attention set is empty

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

Michael Lippautz (Gerrit)

unread,

10:25 AM (2 hours ago) 10:25 AM

to Chromium LUCI CQ, Mikihito Matsuura, Kalvin Lee, Keishi Hattori, chromium...@chromium.org, Kentaro Hara, bartek...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, lize...@chromium.org, lizeb...@chromium.org, wfh+...@chromium.org

Michael Lippautz added 1 comment

Patchset-level comments

File-level comment, Patchset 7 (Latest):

Michael Lippautz . unresolved

This also broke V8 downstream and blocked a DEPS roll. How does PA ensure that it doesn't block embedders? Are we holding things wrong in V8?

In fact, we had to switch to an `internal` version now: https://chromium-review.googlesource.com/c/v8/v8/+/7414928

Open in Gerrit

Related details

Attention set is empty

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

Reply all

Reply to author

Forward

[PA] Parametrize Empty Slot Span Ring Buffer size [chromium/src : main]

Mikihito Matsuura (Gerrit)

Mikihito Matsuura added 1 comment

Related details

Mikihito Matsuura (Gerrit)

Mikihito Matsuura voted and added 1 comment

Votes added by Mikihito Matsuura

1 comment

Related details

Kalvin Lee (Gerrit)

Kalvin Lee voted and added 2 comments

Votes added by Kalvin Lee

2 comments

Related details

Mikihito Matsuura (Gerrit)

Mikihito Matsuura voted Commit-Queue+2

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

Change information

Michael Lippautz (Gerrit)

Michael Lippautz added 1 comment

Related details

Michael Lippautz (Gerrit)

Michael Lippautz added 1 comment

Related details