[go] container/heap: use sort.Interface where possible

4 views
Skip to first unread message

Gerrit Bot (Gerrit)

unread,
Feb 27, 2024, 12:42:09 PM2/27/24
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Gerrit Bot has uploaded the change for review

Commit message

container/heap: use sort.Interface where possible

Functions heap.Pop(), heap.Push() and heap.Remove() use type heap.Interface. This type is equal to sort.Interface, with Push() and Pop() methods added. This is necessary, because these functions affect the size of the resulting heap.

Functions heap.Fix() and heap.Init() are different. They may compare and swap elements in the underlying container, but never alter its size. It is therefore possible to just use sort.Interface. This would make these functions a lot more pleasant to use in situations where you know the number of elements in the heap is fixed.

Consider the case where you use these functions to merge the results of n infinitely long monotonically increasing sequences. For example, logs received from multiple sources that need to be turned into a single stream of events sorted by time. This can now be implemented as follows:

type Value ...
type Sequence <-chan Value

type PeekedSequence struct {
First Value
Remainder Sequence
}

type SequenceHeap []PeekedSequence

func (h SequenceHeap) Len() int { return len(h) }
func (h SequenceHeap) Less(i, j int) bool { return h[i].First < h[j].First }
func (h SequenceHeap) Swap(i, j int) { h[i], h[j] = h[j], h[i] }

func MergeSequences(sequences []Sequence, out chan<- Value) {
h := make(SequenceHeap, 0, len(sequences))
for _, s := range sequences {
h = append(h, PeekedSequence{
First: <-s,
Remainder: s,
})
}
heap.Init(h)
for {
out <- h[0].First
h[0].First = <-h[0].Remainder
heap.Fix(h, 0)
}
}

Before this change was made, SequenceHeap had to provide Pop() and Push() methods that are known to be unused.
Change-Id: I90639a5e3bffdfe724d409e124e4a4c0d9ce5fec
GitHub-Last-Rev: cff8a71275241b0a4caae658ce91999301d1a42e
GitHub-Pull-Request: golang/go#65967

Change diff

diff --git a/src/container/heap/heap.go b/src/container/heap/heap.go
index 3ad218e..56a268c 100644
--- a/src/container/heap/heap.go
+++ b/src/container/heap/heap.go
@@ -38,7 +38,7 @@
// Init is idempotent with respect to the heap invariants
// and may be called whenever the heap invariants may have been invalidated.
// The complexity is O(n) where n = h.Len().
-func Init(h Interface) {
+func Init(h sort.Interface) {
// heapify
n := h.Len()
for i := n/2 - 1; i >= 0; i-- {
@@ -80,13 +80,13 @@
// Changing the value of the element at index i and then calling Fix is equivalent to,
// but less expensive than, calling [Remove](h, i) followed by a Push of the new value.
// The complexity is O(log n) where n = h.Len().
-func Fix(h Interface, i int) {
+func Fix(h sort.Interface, i int) {
if !down(h, i, h.Len()) {
up(h, i)
}
}

-func up(h Interface, j int) {
+func up(h sort.Interface, j int) {
for {
i := (j - 1) / 2 // parent
if i == j || !h.Less(j, i) {
@@ -97,7 +97,7 @@
}
}

-func down(h Interface, i0, n int) bool {
+func down(h sort.Interface, i0, n int) bool {
i := i0
for {
j1 := 2*i + 1

Change information

Files:
  • M src/container/heap/heap.go
Change size: XS
Delta: 1 file changed, 4 insertions(+), 4 deletions(-)
Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement is not satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: newchange
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I90639a5e3bffdfe724d409e124e4a4c0d9ce5fec
Gerrit-Change-Number: 567475
Gerrit-PatchSet: 1
Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
unsatisfied_requirement
satisfied_requirement
open
diffy

Gopher Robot (Gerrit)

unread,
Feb 27, 2024, 12:42:11 PM2/27/24
to Gerrit Bot, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Gopher Robot added 1 comment

Patchset-level comments
File-level comment, Patchset 1 (Latest):
Gopher Robot . unresolved

I spotted some possible problems.

These findings are based on simple heuristics. If a finding appears wrong, briefly reply here saying so. Otherwise, please address any problems and update the GitHub PR. When complete, mark this comment as 'Done' and click the [blue 'Reply' button](https://go.dev/wiki/GerritBot#i-left-a-reply-to-a-comment-in-gerrit-but-no-one-but-me-can-see-it) above.

Possible problems detected:
1. Lines in the commit message should be wrapped at ~76 characters unless needed for things like URLs or tables. You have a 327 character line.
2. You usually need to reference a bug number for all but trivial or cosmetic fixes. For this repo, the format is usually 'Fixes #12345' or 'Updates #12345' at the end of the commit message. Should you have a bug reference?

The commit title and commit message body come from the GitHub PR title and description, and must be edited in the GitHub web interface (not via git). For instructions, see [here](https://go.dev/wiki/GerritBot/#how-does-gerritbot-determine-the-final-commit-message). For guidelines on commit messages for the Go project, see [here](https://go.dev/doc/contribute#commit_messages).


(In general for Gerrit code reviews, the change author is expected to [log in to Gerrit](https://go-review.googlesource.com/login/) with a Gmail or other Google account and then close out each piece of feedback by marking it as 'Done' if implemented as suggested or otherwise reply to each review comment. See the [Review](https://go.dev/doc/contribute#review) section of the Contributing Guide for details.)

Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement is not satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: I90639a5e3bffdfe724d409e124e4a4c0d9ce5fec
    Gerrit-Change-Number: 567475
    Gerrit-PatchSet: 1
    Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-Comment-Date: Tue, 27 Feb 2024 17:42:06 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    unsatisfied_requirement
    open
    diffy

    Gerrit Bot (Gerrit)

    unread,
    Feb 27, 2024, 12:48:53 PM2/27/24
    to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

    Gerrit Bot uploaded new patchset

    Gerrit Bot uploaded patch set #2 to this change.
    Open in Gerrit

    Related details

    Attention set is empty
    Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement is not satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: newpatchset
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: I90639a5e3bffdfe724d409e124e4a4c0d9ce5fec
    Gerrit-Change-Number: 567475
    Gerrit-PatchSet: 2
    unsatisfied_requirement
    open
    diffy

    Robert Griesemer (Gerrit)

    unread,
    Feb 27, 2024, 4:54:19 PM2/27/24
    to Gerrit Bot, goph...@pubsubhelper.golang.org, Robert Griesemer, Gopher Robot, golang-co...@googlegroups.com
    Attention needed from Robert Griesemer

    Robert Griesemer added 1 comment

    Patchset-level comments
    File-level comment, Patchset 2 (Latest):
    Robert Griesemer . resolved

    Thanks, but we cannot make this change in a backward-compatible way: if some code depends on the current signature of, say heap.Init, specifically that the argument is of type heap.Interface, changing it to sort.Interface changes that signature type which in turn may break clients (unlikely but possible).

    Furthermore, we're not making changes to container/* packages except for bug fixes. These packages need to be essentially rewritten, with generic APIs. Specifically, container/heap is hard to use and needs to be replaced eventually.

    You're of course welcome to copy this code and change it as you please for your own purposes.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Robert Griesemer
    Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement is not satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: I90639a5e3bffdfe724d409e124e4a4c0d9ce5fec
    Gerrit-Change-Number: 567475
    Gerrit-PatchSet: 2
    Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
    Gerrit-Reviewer: Robert Griesemer <g...@golang.org>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-CC: Robert Griesemer <g...@google.com>
    Gerrit-Attention: Robert Griesemer <g...@golang.org>
    Gerrit-Comment-Date: Tue, 27 Feb 2024 21:54:13 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    unsatisfied_requirement
    open
    diffy

    Sean Liao (Gerrit)

    unread,
    6:04 PM (5 hours ago) 6:04 PM
    to Gerrit Bot, goph...@pubsubhelper.golang.org, Robert Griesemer, Robert Griesemer, Gopher Robot, golang-co...@googlegroups.com

    Sean Liao abandoned this change.

    View Change

    Abandoned

    Sean Liao abandoned this change

    Related details

    Attention set is empty
    Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement is not satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: abandon
    unsatisfied_requirement
    open
    diffy
    Reply all
    Reply to author
    Forward
    0 new messages