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

runtime: refactor into separate subpackages #11647

Open
michaelmatloob opened this issue Jul 9, 2015 · 14 comments
Open

runtime: refactor into separate subpackages #11647

michaelmatloob opened this issue Jul 9, 2015 · 14 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unplanned

Comments

@michaelmatloob
Copy link
Contributor

Now that the runtime is Go code, we should be able to break it apart into multiple packages.

Among other things we should consider putting locking, the gc, and special symbols in separate packages.
@michaelmatloob michaelmatloob changed the title refactor the runtime into separate packages runtime: refactor the into separate subpackages Jul 9, 2015
@ianlancetaylor ianlancetaylor changed the title runtime: refactor the into separate subpackages runtime: refactor into separate subpackages Jul 10, 2015
@ianlancetaylor ianlancetaylor modified the milestones: Go1.6, Go1.6Early Jul 10, 2015
@robpike
Copy link
Contributor

robpike commented Jul 10, 2015

Why, exactly?

@michaelmatloob
Copy link
Contributor Author

Now that the runtime is written in Go, it's worth taking the time to consider whether it could be better organized if it were split into packages. I think it could. Splitting the runtime into packages could also help improve the readability, in the style sense, of the code. That would make the runtime more accessible to new contributors.

It was also suggested by crawshaw that putting certain parts of the runtime into their own packages would make it easier to stub out or replace parts of the runtime. That would be helpful when bootstrapping a new architecture or os, where all of the runtime is not available, or in creating a modified runtime that would run without an os.

@josharian
Copy link
Contributor

I'm in favor of splitting up the runtime, at least insofar as it is mostly just movement of existing code. This has the following advantages:

  • Makes the structure and components of the runtime more obvious. There really are several basically standalone pieces.
  • Faster to run runtime tests when making modifications. (This is a pain point; the runtime tests are slow.)
  • Easier to do safe refactoring and experimentation.
  • Having a runtime/internal/magic package will make it more obvious where the runtime touches the compiler.

@michaelmatloob
Copy link
Contributor Author

CL https://golang.org/cl/14204 updates this issue.

@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Nov 12, 2015
@matloob
runtime: break out system-specific constants into package sys
432cb66 
runtime/internal/sys will hold system-, architecture- and config-
specific constants.

Updates #11647

Change-Id: I6db29c312556087a42e8d2bdd9af40d157c56b54
Reviewed-on: https://go-review.googlesource.com/16817
Reviewed-by: Russ Cox <rsc@golang.org>
@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Nov 12, 2015
@matloob
runtime: move arch_mips64(le)?.go into runtime/internal/sys
d3498c5 
Somehow these were left out of the orignial CL.

Updates #11647

Change-Id: I058a30eaa25fbb72d60e7fb6bc9ff0a3b54fdb2a
Reviewed-on: https://go-review.googlesource.com/16870
Reviewed-by: Minux Ma <minux@golang.org>
@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Nov 12, 2015
@matloob
runtime: delete runtime/internal/atomic/textflag.h
7c38ae0 
As per mdempsky's comment on golang.org/cl/14204, textflag.h is
copied to the includes dir by cmd/dist, and the copy in
runtime/internal/atomic is not actually being used.

Updates #11647

Change-Id: Ie95c08903a9df54cea4c70ee9d5291176f7b5609
Reviewed-on: https://go-review.googlesource.com/16871
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
@rsc
Copy link
Contributor

rsc commented Nov 24, 2015

We got a couple subpackages in for Go 1.6. More work in Go 1.7.

@rsc rsc modified the milestones: Go1.7Early, Go1.6Early Nov 24, 2015
@bradfitz bradfitz modified the milestones: Go1.8, Go1.7Early May 6, 2016
@josharian
Copy link
Contributor

Another advantage to this: More of the runtime could be build concurrently. It's not super slow to build, but it always runs first, and linearly, since everything else depends on it. See the graph at #15734.

@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 11, 2016
@bradfitz
Copy link
Contributor

Bumping this to Go 1.9. Not sure anything happened in Go 1.8 but it seems too late now.

@bradfitz bradfitz modified the milestones: Go1.9, Go1.8 Oct 28, 2016
@gopherbot
Copy link

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

@aclements aclements modified the milestones: Go1.10, Go1.9 Jun 9, 2017
@rsc rsc removed this from the Go1.10 milestone Nov 22, 2017
@rsc rsc added this to the Go1.11 milestone Nov 22, 2017
@aclements aclements modified the milestones: Go1.11, Unplanned Jul 3, 2018
@gopherbot
Copy link

Change https://golang.org/cl/171702 mentions this issue: devapp/owners: remove josharian from runtime

gopherbot pushed a commit to golang/build that referenced this issue Apr 12, 2019
@josharian
devapp/owners: remove josharian from runtime
bbcc503 
The runtime package contains multitudes.
I only care about a few of them.
The signal to noise ratio on changes is currently too low.

I hope that we will someday resume work on golang/go#11647,
and/or implement some devapp filters. (Keyword-based? File-based?)

Until then, I'm tapping out.

Change-Id: Ib3baabb44f3a9ee48d78dcb8234c3752e0e42278
Reviewed-on: https://go-review.googlesource.com/c/build/+/171702
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
@bcmills
Copy link
Contributor

bcmills commented Apr 12, 2019

At the very least, if there are parts of runtime that are not tightly coupled to the implementation details of the compiler, it would probably help maintainers of other compliers (such as TinyGo) to split those parts into separate subpackages. (See #31330.)

@josharian
Copy link
Contributor

Another advantage to doing this: Any parts of the runtime not involved in scheduling can have their race/msan instrumentation done by the compiler rather than manually.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 7, 2022
@matloob matloob removed their assignment Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests