New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
runtime: remove no-op pointer writes in treap rotations #28420
Conversation
This PR (HEAD: 4b81a79) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/go/+/144999 to see it. Tip: You can toggle comments from me using the |
Message from Gerrit User 13550: Patch Set 1: Code-Review-1 This CL needs to explain why the code is unnecessary. Perhaps I'm missing something obvious, but the code doesn't seem unnecessary to me by just reading the commit message and diff. Please don’t reply on this GitHub thread. Visit golang.org/cl/144999. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zhiqiangxu I might need to explain why the code is unnecessary.
@@ -373,19 +373,11 @@ Found: | |||
func (root *semaRoot) rotateLeft(x *sudog) { | |||
// p -> (x a (y b c)) | |||
p := x.parent | |||
a, y := x.prev, x.next |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked the whole file but the code not seems to be unnecessary. Even though, I think the code remain here is to exemplify what was the logic here before they code this?
|
||
y.prev = x | ||
x.parent = y | ||
y.next = c |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did they even use this before? What was the logic?
Message from Rick Hudson: Patch Set 1: Code-Review+2
The point is that if you want to rotate left the parent x with the offspring y, ie change Please don’t reply on this GitHub thread. Visit golang.org/cl/144999. |
Message from Daniel Martí: Patch Set 1:
Fair enough. I'd still include this clarification in the commit message, though :) Please don’t reply on this GitHub thread. Visit golang.org/cl/144999. |
Message from Josh Bleecher Snyder: Patch Set 1: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/144999. |
Message from Rick Hudson: Patch Set 1: Run-TryBot+1 Trybots Please don’t reply on this GitHub thread. Visit golang.org/cl/144999. |
Message from Gobot Gobot: Patch Set 1: TryBots beginning. Status page: https://farmer.golang.org/try?commit=ee47dcf1 Please don’t reply on this GitHub thread. Visit golang.org/cl/144999. |
Message from Gobot Gobot: Patch Set 1: Build is still in progress... Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report. Please don’t reply on this GitHub thread. Visit golang.org/cl/144999. |
Message from Gobot Gobot: Patch Set 1: TryBot-Result-1 1 of 19 TryBots failed: Consult https://build.golang.org/ to see whether they are new failures. Please don’t reply on this GitHub thread. Visit golang.org/cl/144999. |
Message from Rick Hudson: Patch Set 1: Trybot fail seems unrelated, try again. Please don’t reply on this GitHub thread. Visit golang.org/cl/144999. |
Message from Rick Hudson: Patch Set 1:
Please don’t reply on this GitHub thread. Visit golang.org/cl/144999. |
Message from Russ Cox: Patch Set 1: Code-Review-2 Please change the first line of the commit message to the standard form, with a prefix giving the package name and a clearer summary of the change:
Please don’t reply on this GitHub thread. Visit golang.org/cl/144999. |
Message from Russ Cox: Patch Set 1: Also please update your Author line to use a full name. Thanks. Please don’t reply on this GitHub thread. Visit golang.org/cl/144999. |
53bd915
to
6139019
Compare
Message from Brad Fitzpatrick: Patch Set 3: Patch Set 2 was rebased Please don’t reply on this GitHub thread. Visit golang.org/cl/144999. |
Message from Brad Fitzpatrick: Patch Set 3: Run-TryBot+1 Please don’t reply on this GitHub thread. Visit golang.org/cl/144999. |
Message from Gobot Gobot: Patch Set 3: TryBots beginning. Status page: https://farmer.golang.org/try?commit=c73389c0 Please don’t reply on this GitHub thread. Visit golang.org/cl/144999. |
Message from Gobot Gobot: Patch Set 3: TryBot-Result+1 TryBots are happy. Please don’t reply on this GitHub thread. Visit golang.org/cl/144999. |
This PR is being closed because golang.org/cl/144999 has been merged. |
Change-Id: If5a272f331fe9da09467efedd0231a4ce34db0f8 GitHub-Last-Rev: 4b81a79 GitHub-Pull-Request: #28420 Reviewed-on: https://go-review.googlesource.com/c/go/+/144999 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rick Hudson <rlh@golang.org>
No description provided.