Skip to content
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

Jump to bottom

cmd/compile: remove writebarrierfat-referencing code from order.go #18687

Closed
mdempsky opened this issue Jan 17, 2017 · 7 comments
Closed

cmd/compile: remove writebarrierfat-referencing code from order.go #18687

mdempsky opened this issue Jan 17, 2017 · 7 comments
Labels
FrozenDueToAge
Milestone
Go1.9Early

Comments

@mdempsky
Copy link
Member

In CL 21242 (deb83d0), I tried to remove some code that referenced the then (and now still) non-existent writebarrierfat function. However, it caused net/http's TestServerEmptyBodyRace_h2 test to mysteriously become flaky.

This seems to be fixed now, and bisecting identifies CL 23522 (42da35c) as the fix.

It should be safe to remove for 1.9.
@mdempsky mdempsky added this to the Go1.9Early milestone Jan 17, 2017
@mdempsky mdempsky self-assigned this Jan 17, 2017
@gopherbot
Copy link

CL https://golang.org/cl/35243 mentions this issue.

@cherrymui
Copy link
Member

Coincidentally I was doing the same thing in https://go-review.googlesource.com/c/35261/ yesterday, and it seems to work ok.

@mdempsky
Copy link
Member Author

@cherrymui Yeah, that was the next step I had in mind after removing that bit of code in order.go. If you've already got it done, we can go with your CL when tree reopens.

I just wanted to file the issue since I've poked at that code a few times, but kept forgetting to write down my notes somewhere and end up reinvestigating it. :)

@mdempsky
Copy link
Member Author

mdempsky commented Feb 7, 2017

@cherrymui Do you plan on looking at this still?

@cherrymui
Copy link
Member

@mdempsky Yeah, I'll rebase my CL to tip and send for review (although if we address issue #18872 then the optimization of zeroing part may be not necessary?).

@mdempsky
Copy link
Member Author

mdempsky commented Feb 7, 2017

@cherrymui SGTM. I'm mostly interested in getting rid of that weird bit in order.go. :)

@gopherbot
Copy link

CL https://golang.org/cl/35261 mentions this issue.

@golang golang locked and limited conversation to collaborators Feb 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.9Early
Development

No branches or pull requests
3 participants
@mdempsky @gopherbot @cherrymui