[memory-reducer] Add SimpleMemoryReducer [v8/v8 : main]

0 views
Skip to first unread message

Thiabaud Engelbrecht (Gerrit)

unread,
Oct 16, 2025, 12:48:01 PM (7 days ago) Oct 16
to Etienne Pierre-Doray, Hannes Payer, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Etienne Pierre-Doray

Thiabaud Engelbrecht added 3 comments

File src/heap/memory-reducer.cc
Line 292, Patchset 1: task_runner_->PostDelayedTask(
Etienne Pierre-Doray . resolved

We need to avoid double posting:
Either we cancel the previous posted task, or we avoid reposting (that seems easier if canceling is too complicated)

Thiabaud Engelbrecht

Done

Line 302, Patchset 1:
Etienne Pierre-Doray . resolved

We want to SampleAllocation() too here

Thiabaud Engelbrecht

Done

Line 307, Patchset 1: if (!started) {
Etienne Pierre-Doray . resolved

If it does start, we want to remember that we did on NotifyMarkCompact (called when gc ends)
On a NotifyMarkCompact() event that was started by SimpleMemoryReducer, we don't want to schedule the timer.

Thiabaud Engelbrecht

Done

Open in Gerrit

Related details

Attention is currently required from:
  • Etienne Pierre-Doray
Submit Requirements:
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: v8/v8
Gerrit-Branch: main
Gerrit-Change-Id: Ia8bcdd528ee3c9e457acc415f9d7611791ac9d3b
Gerrit-Change-Number: 7042151
Gerrit-PatchSet: 2
Gerrit-Owner: Thiabaud Engelbrecht <thia...@google.com>
Gerrit-Reviewer: Etienne Pierre-Doray <etie...@chromium.org>
Gerrit-CC: Hannes Payer <hpa...@chromium.org>
Gerrit-Attention: Etienne Pierre-Doray <etie...@chromium.org>
Gerrit-Comment-Date: Thu, 16 Oct 2025 16:47:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Etienne Pierre-Doray <etie...@chromium.org>
unsatisfied_requirement
open
diffy

Etienne Pierre-Doray (Gerrit)

unread,
Oct 22, 2025, 10:30:41 AM (yesterday) Oct 22
to Thiabaud Engelbrecht, V8 LUCI CQ, AyeAye, Hannes Payer, v8-flag...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Thiabaud Engelbrecht

Etienne Pierre-Doray added 3 comments

File src/heap/memory-reducer.h
Line 258, Patchset 7 (Latest): bool is_running_ = false;
Etienne Pierre-Doray . unresolved

Nit: `is_scheduled`, will help disambiguate with `triggered_mark_compact` (see my other commend)

File src/heap/memory-reducer.cc
Line 289, Patchset 7 (Latest): ScheduleTimer(kDelayMs, true);
Etienne Pierre-Doray . unresolved

We should prevent ScheduleTimer if NotifyPossibleGarbage() was triggered by our own GC.
You will probably have to add a `bool triggered_mark_compact` that reset here, and if true you don't call ScheduleTimer.

Line 310, Patchset 7 (Latest): // Leave some room for precision error in task scheduler.
const double kSlackMs = 100;
Etienne Pierre-Doray . unresolved

I doubt that's necessary and can't really figure out why that was introduced in the original MemoryReducer.

Open in Gerrit

Related details

Attention is currently required from:
  • Thiabaud Engelbrecht
Submit Requirements:
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: v8/v8
    Gerrit-Branch: main
    Gerrit-Change-Id: Ia8bcdd528ee3c9e457acc415f9d7611791ac9d3b
    Gerrit-Change-Number: 7042151
    Gerrit-PatchSet: 7
    Gerrit-Owner: Thiabaud Engelbrecht <thia...@google.com>
    Gerrit-Reviewer: Etienne Pierre-Doray <etie...@chromium.org>
    Gerrit-Reviewer: Thiabaud Engelbrecht <thia...@google.com>
    Gerrit-CC: Hannes Payer <hpa...@chromium.org>
    Gerrit-Attention: Thiabaud Engelbrecht <thia...@google.com>
    Gerrit-Comment-Date: Wed, 22 Oct 2025 14:30:38 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    unsatisfied_requirement
    open
    diffy

    Thiabaud Engelbrecht (Gerrit)

    unread,
    Oct 22, 2025, 11:54:00 AM (24 hours ago) Oct 22
    to V8 LUCI CQ, AyeAye, Etienne Pierre-Doray, Hannes Payer, v8-flag...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
    Attention needed from Etienne Pierre-Doray

    Thiabaud Engelbrecht added 3 comments

    File src/heap/memory-reducer.h
    Line 258, Patchset 7: bool is_running_ = false;
    Etienne Pierre-Doray . resolved

    Nit: `is_scheduled`, will help disambiguate with `triggered_mark_compact` (see my other commend)

    Thiabaud Engelbrecht

    Done

    File src/heap/memory-reducer.cc
    Line 289, Patchset 7: ScheduleTimer(kDelayMs, true);
    Etienne Pierre-Doray . resolved

    We should prevent ScheduleTimer if NotifyPossibleGarbage() was triggered by our own GC.
    You will probably have to add a `bool triggered_mark_compact` that reset here, and if true you don't call ScheduleTimer.

    Thiabaud Engelbrecht

    Done

    Line 310, Patchset 7: // Leave some room for precision error in task scheduler.

    const double kSlackMs = 100;
    Etienne Pierre-Doray . resolved

    I doubt that's necessary and can't really figure out why that was introduced in the original MemoryReducer.

    Thiabaud Engelbrecht

    Done

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Etienne Pierre-Doray
    Submit Requirements:
      • requirement is not satisfiedCode-Owners
      • requirement is not satisfiedCode-Review
      • requirement is not satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: v8/v8
      Gerrit-Branch: main
      Gerrit-Change-Id: Ia8bcdd528ee3c9e457acc415f9d7611791ac9d3b
      Gerrit-Change-Number: 7042151
      Gerrit-PatchSet: 9
      Gerrit-Owner: Thiabaud Engelbrecht <thia...@google.com>
      Gerrit-Reviewer: Etienne Pierre-Doray <etie...@chromium.org>
      Gerrit-Reviewer: Thiabaud Engelbrecht <thia...@google.com>
      Gerrit-CC: Hannes Payer <hpa...@chromium.org>
      Gerrit-Attention: Etienne Pierre-Doray <etie...@chromium.org>
      Gerrit-Comment-Date: Wed, 22 Oct 2025 15:53:56 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Etienne Pierre-Doray <etie...@chromium.org>
      unsatisfied_requirement
      open
      diffy

      Etienne Pierre-Doray (Gerrit)

      unread,
      Oct 22, 2025, 4:21:44 PM (19 hours ago) Oct 22
      to Thiabaud Engelbrecht, V8 LUCI CQ, AyeAye, Hannes Payer, v8-flag...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
      Attention needed from Thiabaud Engelbrecht

      Etienne Pierre-Doray added 2 comments

      File test/cctest/heap/test-incremental-marking.cc
      Line 155, Patchset 9 (Latest): auto memory_reducer =
      reinterpret_cast<SimpleMemoryReducer*>(heap->memory_reducer());
      Etienne Pierre-Doray . unresolved

      Is this created after memory_reducer_simple = true now?
      How do we know this is indeed a SimpleMemoryReducer?

      Line 161, Patchset 9 (Latest): memory_reducer->AllowRunning();
      Etienne Pierre-Doray . unresolved

      Why is this needs - Do you know what triggers the memory reducer otherwise?

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Thiabaud Engelbrecht
      Submit Requirements:
        • requirement is not satisfiedCode-Owners
        • requirement is not satisfiedCode-Review
        • requirement is not satisfiedNo-Unresolved-Comments
        • requirement is not satisfiedReview-Enforcement
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: v8/v8
        Gerrit-Branch: main
        Gerrit-Change-Id: Ia8bcdd528ee3c9e457acc415f9d7611791ac9d3b
        Gerrit-Change-Number: 7042151
        Gerrit-PatchSet: 9
        Gerrit-Owner: Thiabaud Engelbrecht <thia...@google.com>
        Gerrit-Reviewer: Etienne Pierre-Doray <etie...@chromium.org>
        Gerrit-Reviewer: Thiabaud Engelbrecht <thia...@google.com>
        Gerrit-CC: Hannes Payer <hpa...@chromium.org>
        Gerrit-Attention: Thiabaud Engelbrecht <thia...@google.com>
        Gerrit-Comment-Date: Wed, 22 Oct 2025 20:21:41 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        unsatisfied_requirement
        open
        diffy
        Reply all
        Reply to author
        Forward
        0 new messages