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

cmd/link: merge dev.link into master branch for Go 1.16 early #40703

Closed
aclements opened this issue Aug 11, 2020 · 8 comments
Closed

cmd/link: merge dev.link into master branch for Go 1.16 early #40703

aclements opened this issue Aug 11, 2020 · 8 comments
Labels
early-in-cycle A change that should be done early in the 3 month dev cycle. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@aclements
Copy link
Member

This is a tracking issue for merging the dev.link branch into master for Go 1.16. A great deal of work has happened on dev.link to further improve the linker since Go 1.15, and we plan to merge this as soon as the 1.16 tree opens in order to avoid conflicts. At this point, we're wrapping up a few final changes and don't expect any more major changes to the linker in the near term.

While this is a large change, we propose to do the merge without a feature flag. Instead, we believe the following are sufficient risk mitigation:

  1. By merging as soon as Go 1.16 opens and not making further major changes to the linker, the linker changes will have maximal soak time. The changes are involved enough that we believe fixing any new bugs at this point would be preferable to backing them out, and maximizing the soak time also maximizes the time we have to fix bugs. This soak time will include regular testing across Google's code base from the start.

  2. Failures caused by the linker tend to be deterministic and are often not subtle, so testing is actually quite effective (in contrast with, say, runtime bugs, which tend to be non-deterministic and subtle). We theorized testing would be effective with the linker changes that went into 1.15 and, indeed, there were very few bugs and they were all relatively straightforward to debug.

  3. Since this work has been happening on a branch, we've been able to monitor the build dashboard throughout development. Hence, the linker changes have already had significant automated testing, and we've been able to keep the branch quite stable throughout.

In contrast, we believe attempting to feature flag this work would be so involved as to actually increase risk and add significant testing burden, as there's no "obviously correct" fallback (unlike how Go 1.15 could fall back to the 1.14 linker).

/cc @rsc @andybons @jeremyfaller @cherrymui @thanm

@aclements aclements added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Aug 11, 2020
@aclements aclements added this to the Go1.16 milestone Aug 11, 2020
@andybons
Copy link
Member

This seems reasonable to me. Let’s plan to merge tomorrow when we open the tree to large changes.

@andybons andybons added early-in-cycle A change that should be done early in the 3 month dev cycle. NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Aug 12, 2020
@gopherbot
Copy link

Change https://golang.org/cl/248197 mentions this issue: cmd: merge branch 'dev.link' into master

@dmitshur
Copy link
Contributor

This issue is currently labeled as early-in-cycle for Go 1.16.
That time is now, so this is a friendly ping so the issue is looked at again.

@thanm
Copy link
Contributor

thanm commented Aug 12, 2020

Thanks Dmitri. This is being actively worked on. I am waiting on a couple more CLs from Jeremy before finalizing the merge.

@gopherbot
Copy link

Change https://golang.org/cl/248200 mentions this issue: cmd: merge branch 'dev.link' into master

@thanm
Copy link
Contributor

thanm commented Aug 12, 2020

Hmm, I think I may need to generate a new CL instead of hijacking the old one. Stay tuned.

@gopherbot
Copy link

Change https://golang.org/cl/248238 mentions this issue: doc: add a release notes blurb on 1.16 linker improvements

@gopherbot
Copy link

Change https://golang.org/cl/248318 mentions this issue: cmd: merge branch 'dev.link' into master

gopherbot pushed a commit that referenced this issue Aug 12, 2020
In the dev.link branch we have continued developing the new object
file format support and the linker improvements described in
https://golang.org/s/better-linker . Since the last merge (May 1st
2020), more progress has been made to improve the new linker, with
improvements on both linker speed and memory usage.

Fixes #40703.

Change-Id: I9924ea88d981845c3a40ec8c25820120fc21c003
gopherbot pushed a commit that referenced this issue Aug 13, 2020
Add a draft version of a blurb on improvements to the linker. This
will need to be finalized later in the release since there are still
some additional changes to be made to the linker in 1.16.

Updates #40703.

Change-Id: Id85c7e129071cc2faacb09c53a2968bd52b0a7b4
Reviewed-on: https://go-review.googlesource.com/c/go/+/248238
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Austin Clements <austin@google.com>
@golang golang locked and limited conversation to collaborators Aug 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
early-in-cycle A change that should be done early in the 3 month dev cycle. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants