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.

Sign up for GitHub

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: move internals to internal/runtime #65355

Open
prattmic opened this issue Jan 29, 2024 · 16 comments
Open

runtime: move internals to internal/runtime #65355

prattmic opened this issue Jan 29, 2024 · 16 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
Backlog

Comments

@prattmic
Copy link
Member

Right now package runtime appears to serve two purposes:

  • It contains most of the implementation of the Go runtime.
  • And, it exposes the world-public API for various runtime-related functionality (runtime.GOMAXPROCS, etc).

At first glance, this seems perfectly normal and reasonable, however there is actually a third purpose:

  • Expose std-internal APIs that should not be world-public. e.g., runtime.semacquire for sync.Mutex or runtime.netpollopen for os and net.

These APIs should not be public to the whole world as they are implementation details that may change. However, with the current layout there is no good way to expose these internal APIs, so we must use //go:linkname between packages, and in some cases copy entire type definitions between packages (#64549).

If we move most of the runtime implementation to internal/runtime, then it could expose a richer API surface useful to other packages in std, but nothing outside of std would be able to access them due to the rules of internal. Package runtime would remain to provide the same API, but it would be much smaller and simply import internal/runtime for most functionality.

In moving, there are also opportunities for cleanup along the lines of #51087. e.g., perhaps the timer implementation is in internal/runtime/timer, etc. Though this move isn't really a prerequisite for that; we could do that in runtime/internal/timer as well.

This change would technically be solely an implementation detail, though in practice there are likely symbols that folks depend on anyway, either via linkname (https://go.dev/cl/548235) or implicitly by looking for in profiles, stack traces, etc.

cc @golang/runtime
@prattmic prattmic added the NeedsFix The path to resolution is known, but the work has not been done. label Jan 29, 2024
@prattmic prattmic added this to the Backlog milestone Jan 29, 2024
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jan 29, 2024
@mauri870
Copy link
Member

I really like the idea of having a internal/runtime package. The runtime code always felt too "convoluted" for me. It grew a lot with time to the point it is now a sum of numerous assembly files, internal code for the GC and auxiliary runtime stuff, race, asan, msan code, and buried in the middle of all this the public runtime apis. I believe having an internal package dedicated to these bits would hopefully clean runtime enough that it will be easier to navigate and more inlined with the other standard library packages.

@qiulaidongfeng qiulaidongfeng mentioned this issue Jan 30, 2024
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/559076 mentions this issue: runtime,time: clean duplicate definitions time.runtimeTimer and runtime.timer

qiulaidongfeng added a commit to qiulaidongfeng/go that referenced this issue Jan 31, 2024
@qiulaidongfeng
runtime,reflect: clean duplicate definitions runtimeSelect
890e31c 
For golang#65355

Change-Id: I32f37b2971996d50451505e412157dd49b29f581
@qiulaidongfeng qiulaidongfeng mentioned this issue Jan 31, 2024
Open
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/559675 mentions this issue: runtime,reflect: clean duplicate definitions runtimeSelect

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/559715 mentions this issue: runtime: migrate internal/syscall to internal/runtime

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/560155 mentions this issue: runtime: migrate internal/atomic to internal/runtime

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/560136 mentions this issue: syscall: call internal/runtime/syscall.Syscall6 in RawSyscall6

Jun10ng added a commit to Jun10ng/go that referenced this issue Feb 1, 2024
@Jun10ng
runtime: clean duplicate function
cb3351d 
Fixes golang#64549
For golang#65355
Jun10ng added a commit to Jun10ng/go that referenced this issue Feb 1, 2024
@Jun10ng
runtime: clean duplicate function
28c2ff5 
Fixes golang#64549
For golang#65355
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/560395 mentions this issue: Clean up duplicate function IssystemGoroutine

Jun10ng added a commit to Jun10ng/go that referenced this issue Feb 2, 2024
@Jun10ng
runtime: clean up duplicate function
7588546 
For golang#65355
@Jun10ng Jun10ng mentioned this issue Feb 2, 2024
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/560596 mentions this issue: runtime: clean up duplicate function isSystemGoroutine

qiulaidongfeng added a commit to qiulaidongfeng/go that referenced this issue Feb 7, 2024
@qiulaidongfeng
runtime,reflect: clean duplicate definitions itab
e92816b 
For golang#65355

Change-Id: If676bffe28ae3f20e4ed15a56993811bee05ef22
@qiulaidongfeng qiulaidongfeng mentioned this issue Feb 7, 2024
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/562296 mentions this issue: runtime,reflect: clean duplicate definitions itab

gopherbot pushed a commit that referenced this issue Feb 21, 2024
@panjf2000 @prattmic
runtime: migrate internal/syscall to internal/runtime
05c0579 
For #65355

Change-Id: I5fefe30dcb520159de565e61dafc74a740fc8730
Reviewed-on: https://go-review.googlesource.com/c/go/+/559715
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
gopherbot pushed a commit that referenced this issue Mar 4, 2024
@panjf2000 @gopherbot
syscall: call internal/runtime/syscall.Syscall6 in RawSyscall6
dff3a00 
For #65355

Change-Id: I9168d9a767e3b2ece65ac6dcab6827ab6f6b11bb
Reviewed-on: https://go-review.googlesource.com/c/go/+/560136
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
gopherbot pushed a commit that referenced this issue Mar 25, 2024
@panjf2000 @gopherbot
runtime: migrate internal/atomic to internal/runtime
4c2b1e0 
For #65355

Change-Id: I65dd090fb99de9b231af2112c5ccb0eb635db2be
Reviewed-on: https://go-review.googlesource.com/c/go/+/560155
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ibrahim Bazoka <ibrahimbazoka729@gmail.com>
Auto-Submit: Emmanuel Odeke <emmanuel@orijtech.com>
qiulaidongfeng added a commit to qiulaidongfeng/go that referenced this issue Apr 20, 2024
@qiulaidongfeng
all: move procPin and procUnpin to internal/runtime
d96cb9b 
For golang#65355

Change-Id: I42c04ade295d2900f55596ad2f223c9c93dd740f
@qiulaidongfeng qiulaidongfeng mentioned this issue Apr 20, 2024
Open
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/580536 mentions this issue: all: move procPin and procUnpin to internal/runtime

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/600435 mentions this issue: runtime,internal: move runtime/internal/math to internal/runtime/math

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/600436 mentions this issue: runtime,internal: move runtime/internal/sys to internal/runtime/sys

gopherbot pushed a commit that referenced this issue Jul 23, 2024
@dr2chase
runtime,internal: move runtime/internal/math to internal/runtime/math
f9eb3e3 
Cleanup and friction reduction.

Updates #65355.

Change-Id: I6c4fcd409d044c00d16561fe9ed2257877d73f5b
Reviewed-on: https://go-review.googlesource.com/c/go/+/600435
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@golang.org>
gopherbot pushed a commit that referenced this issue Jul 23, 2024
@dr2chase
runtime,internal: move runtime/internal/sys to internal/runtime/sys
fc5073b 
Cleanup and friction reduction

For #65355.

Change-Id: Ia14c9dc584a529a35b97801dd3e95b9acc99a511
Reviewed-on: https://go-review.googlesource.com/c/go/+/600436
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@golang.org>
@dmitshur dmitshur moved this from Todo to In Progress in Go Compiler / Runtime Jul 23, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/600595 mentions this issue: cmd/go: fix TestNewReleaseRebuildsStalePackagesInGOPATH for runtime move

gopherbot pushed a commit that referenced this issue Jul 23, 2024
@dmitshur @gopherbot
cmd/go: fix TestNewReleaseRebuildsStalePackagesInGOPATH for runtime move
0d5fc4e 
CL 600436 moved runtime/internal/sys to internal/runtime/sys and updated
TestNewReleaseRebuildsStalePackagesInGOPATH in part accordingly. This is
the other part that's needed for it to pass.

For #65355.

Change-Id: I26cff96c15caf185a4ee2c8fb31ec6c877ab87e2
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/600595
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/595295 mentions this issue: devapp: handle internal/runtime moves

gopherbot pushed a commit to golang/build that referenced this issue Jul 29, 2024
@prattmic @gopherbot
devapp: handle internal/runtime moves
Loading
Loading status checks…
6f08d2f 
There have been several moves recently.

Updates golang/go#65355.

Change-Id: Ie00f2e540bc7e51d1f833dffb587b3ce9954601f
Reviewed-on: https://go-review.googlesource.com/c/build/+/595295
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Michael Pratt <mpratt@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/595116 mentions this issue: cmd/compile: keep internal/runtime packages sorted

gopherbot pushed a commit that referenced this issue Jul 31, 2024
@prattmic @gopherbot
cmd/compile: keep internal/runtime packages sorted
0a4215c 
This is a minor cleanup from CL 600436.

For #65355.

Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest,gotip-darwin-amd64-longtest
Change-Id: I8e27f0c6ba6bd35f4aa2b9d53c394fb5f1eb433d
Reviewed-on: https://go-review.googlesource.com/c/go/+/595116
Reviewed-by: Austin Clements <austin@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@callthingsoff callthingsoff mentioned this issue Mar 7, 2025
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/655515 mentions this issue: runtime/internal: clean up completely

gopherbot pushed a commit that referenced this issue Mar 11, 2025
@callthingsoff @prattmic
runtime/internal: clean up completely
31658ac 
We've been slowly moving packages from runtime/internal to
internal/runtime. For now, runtime/internal only has test packages.

It's a good chance to clean up the references to runtime/internal
in the toolchain.

For #65355.

Change-Id: Ie6f9091a44511d0db9946ea6de7a78d3afe9f063
GitHub-Last-Rev: fad32e2
GitHub-Pull-Request: #72137
Reviewed-on: https://go-review.googlesource.com/c/go/+/655515
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
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
Go Compiler / Runtime
Status: In Progress
Milestone
Backlog
Development

No branches or pull requests
3 participants
@prattmic @gopherbot @mauri870