[go] runtime: allocate physical-page-aligned memory differently

7 views
Skip to first unread message

Michael Knyszek (Gerrit)

unread,
May 20, 2022, 12:36:00 PM5/20/22
to goph...@pubsubhelper.golang.org, Cherry Mui, golang-co...@googlegroups.com

Attention is currently required from: Cherry Mui.

Patch set 1:Run-TryBot +1

View Change

1 comment:

To view, visit change 407502. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: Iefa68317584d73b187634979d730eb30db770bb6
Gerrit-Change-Number: 407502
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Knyszek <mkny...@google.com>
Gerrit-Reviewer: Cherry Mui <cher...@google.com>
Gerrit-Reviewer: Michael Knyszek <mkny...@google.com>
Gerrit-Attention: Cherry Mui <cher...@google.com>
Gerrit-Comment-Date: Fri, 20 May 2022 16:35:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Michael Knyszek (Gerrit)

unread,
May 20, 2022, 12:40:02 PM5/20/22
to goph...@pubsubhelper.golang.org, Gopher Robot, Cherry Mui, golang-co...@googlegroups.com

Attention is currently required from: Cherry Mui.

Patch set 2:Run-TryBot +1

View Change

    To view, visit change 407502. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: Iefa68317584d73b187634979d730eb30db770bb6
    Gerrit-Change-Number: 407502
    Gerrit-PatchSet: 2
    Gerrit-Owner: Michael Knyszek <mkny...@google.com>
    Gerrit-Reviewer: Cherry Mui <cher...@google.com>
    Gerrit-Reviewer: Michael Knyszek <mkny...@google.com>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-Attention: Cherry Mui <cher...@google.com>
    Gerrit-Comment-Date: Fri, 20 May 2022 16:39:57 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    Gerrit-MessageType: comment

    Michael Knyszek (Gerrit)

    unread,
    May 20, 2022, 1:10:47 PM5/20/22
    to goph...@pubsubhelper.golang.org, Gopher Robot, Cherry Mui, golang-co...@googlegroups.com

    Attention is currently required from: Cherry Mui.

    Patch set 3:Run-TryBot +1

    View Change

      To view, visit change 407502. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: go
      Gerrit-Branch: master
      Gerrit-Change-Id: Iefa68317584d73b187634979d730eb30db770bb6
      Gerrit-Change-Number: 407502
      Gerrit-PatchSet: 3
      Gerrit-Owner: Michael Knyszek <mkny...@google.com>
      Gerrit-Reviewer: Cherry Mui <cher...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Michael Knyszek <mkny...@google.com>
      Gerrit-Attention: Cherry Mui <cher...@google.com>
      Gerrit-Comment-Date: Fri, 20 May 2022 17:10:43 +0000

      Cherry Mui (Gerrit)

      unread,
      May 20, 2022, 4:46:24 PM5/20/22
      to Michael Knyszek, goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com

      Attention is currently required from: Michael Knyszek.

      Patch set 3:Code-Review +2

      View Change

      1 comment:

      To view, visit change 407502. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: go
      Gerrit-Branch: master
      Gerrit-Change-Id: Iefa68317584d73b187634979d730eb30db770bb6
      Gerrit-Change-Number: 407502
      Gerrit-PatchSet: 3
      Gerrit-Owner: Michael Knyszek <mkny...@google.com>
      Gerrit-Reviewer: Cherry Mui <cher...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Michael Knyszek <mkny...@google.com>
      Gerrit-Attention: Michael Knyszek <mkny...@google.com>
      Gerrit-Comment-Date: Fri, 20 May 2022 20:46:19 +0000

      Michael Knyszek (Gerrit)

      unread,
      May 20, 2022, 5:54:20 PM5/20/22
      to goph...@pubsubhelper.golang.org, Cherry Mui, Gopher Robot, golang-co...@googlegroups.com

      View Change

      1 comment:

      • Patchset:

        • Patch Set #3:

          SlowBots beginning. Status page: https://farmer.golang. […]

          the trybot somehow still hasn't finished yet, but it's definitely gotten way further than in CI so far, so I'm calling this a win.

      To view, visit change 407502. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: go
      Gerrit-Branch: master
      Gerrit-Change-Id: Iefa68317584d73b187634979d730eb30db770bb6
      Gerrit-Change-Number: 407502
      Gerrit-PatchSet: 3
      Gerrit-Owner: Michael Knyszek <mkny...@google.com>
      Gerrit-Reviewer: Cherry Mui <cher...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Michael Knyszek <mkny...@google.com>
      Gerrit-Comment-Date: Fri, 20 May 2022 21:54:15 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Gopher Robot <go...@golang.org>
      Gerrit-MessageType: comment

      Michael Knyszek (Gerrit)

      unread,
      May 20, 2022, 5:54:26 PM5/20/22
      to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Cherry Mui, Gopher Robot, golang-co...@googlegroups.com

      Michael Knyszek submitted this change.

      View Change


      Approvals: Cherry Mui: Looks good to me, approved Michael Knyszek: Run TryBots
      runtime: allocate physical-page-aligned memory differently

      Currently, physical-page-aligned allocations for stacks (where the
      physical page size is greater than the runtime page size) first
      overallocates some memory, then frees the unaligned portions back to the
      heap.

      However, because allocating via h.pages.alloc causes scavenged bits to
      get cleared, we need to account for that memory correctly in heapFree
      and heapReleased. Currently that is not the case, leading to throws at
      runtime.

      Trying to get that accounting right is complicated, because information
      about exactly which pages were scavenged needs to get plumbed up.
      Instead, find the oversized region first, and then only allocate the
      aligned part. This avoids any accounting issues.

      However, this does come with some performance cost, because we don't
      update searchAddr (which is safe, it just means the next allocation
      potentially must look harder) and we skip the fast path that
      h.pages.alloc has for simplicity.

      Fixes #52682.

      Change-Id: Iefa68317584d73b187634979d730eb30db770bb6
      Reviewed-on: https://go-review.googlesource.com/c/go/+/407502
      Run-TryBot: Michael Knyszek <mkny...@google.com>
      Reviewed-by: Cherry Mui <cher...@google.com>
      ---
      M src/runtime/mheap.go
      1 file changed, 59 insertions(+), 19 deletions(-)

      diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go
      index 2d4d7e3..0910aed 100644
      --- a/src/runtime/mheap.go
      +++ b/src/runtime/mheap.go
      @@ -1157,9 +1157,32 @@

      if needPhysPageAlign {
      // Overallocate by a physical page to allow for later alignment.
      - npages += physPageSize / pageSize
      - }
      + extraPages := physPageSize / pageSize

      + // Find a big enough region first, but then only allocate the
      + // aligned portion. We can't just allocate and then free the
      + // edges because we need to account for scavenged memory, and
      + // that's difficult with alloc.
      + //
      + // Note that we skip updates to searchAddr here. It's OK if
      + // it's stale and higher than normal; it'll operate correctly,
      + // just come with a performance cost.
      + base, _ = h.pages.find(npages + extraPages)
      + if base == 0 {
      + var ok bool
      + growth, ok = h.grow(npages + extraPages)
      + if !ok {
      + unlock(&h.lock)
      + return nil
      + }
      + base, _ = h.pages.find(npages + extraPages)
      + if base == 0 {
      + throw("grew heap, but no adequate free space found")
      + }
      + }
      + base = alignUp(base, physPageSize)
      + scav = h.pages.allocRange(base, npages)
      + }
      if base == 0 {
      // Try to acquire a base address.
      base, scav = h.pages.alloc(npages)
      @@ -1181,23 +1204,6 @@
      // one now that we have the heap lock.
      s = h.allocMSpanLocked()
      }
      -
      - if needPhysPageAlign {
      - allocBase, allocPages := base, npages
      - base = alignUp(allocBase, physPageSize)
      - npages -= physPageSize / pageSize
      -
      - // Return memory around the aligned allocation.
      - spaceBefore := base - allocBase
      - if spaceBefore > 0 {
      - h.pages.free(allocBase, spaceBefore/pageSize, false)
      - }
      - spaceAfter := (allocPages-npages)*pageSize - spaceBefore
      - if spaceAfter > 0 {
      - h.pages.free(base+npages*pageSize, spaceAfter/pageSize, false)
      - }
      - }
      -
      unlock(&h.lock)

      HaveSpan:

      To view, visit change 407502. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: go
      Gerrit-Branch: master
      Gerrit-Change-Id: Iefa68317584d73b187634979d730eb30db770bb6
      Gerrit-Change-Number: 407502
      Gerrit-PatchSet: 4
      Gerrit-Owner: Michael Knyszek <mkny...@google.com>
      Gerrit-Reviewer: Cherry Mui <cher...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Michael Knyszek <mkny...@google.com>
      Gerrit-MessageType: merged
      Reply all
      Reply to author
      Forward
      0 new messages