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/go: add GOEXPERIMENT=cacheprog to let a child process implement the internal action/output cache #59719

Closed
bradfitz opened this issue Apr 19, 2023 · 31 comments

Comments

@bradfitz
Copy link
Contributor

bradfitz commented Apr 19, 2023

The cmd/go tool has great caching support. Unfortunately, its caching only supports filesystem-based caching.

I'd like to do things like hook into GitHub's native caching system at a lower level (instead of the inefficient thing people do now: untarring/tarring GOCACHE archives on every CI run, which is often slower than the CI action itself) and support things like a P2P cache gossip protocol between [trusted] coworkers within a company.

Clearly both those examples aren't realistic to add to cmd/go itself. So instead:

I propose that cmd/go support a GOCACHEPROG=/path/to/program environment variable (akin to GOCACHE=/path/to/dir) where the GOCACHEPROG is run as a child process and cmd/go speaks to it over stdin/stdout, translating the Go tool's internal cache interface, and then the GOCACHEPROG can do whatever caching mechanism/policy it wants.

I talked to @rsc about this once and he didn't seem opposed so I went off and implemented it and it's looking like it's going to be pretty awesome. (demo programs)

Thoughts, objections, etc?

(And preemptively: I have a soft spot for FUSE but FUSE is not an answer; it doesn't work in enough environments like CI test runner environments and it's finicky on basically all platforms but Linux, but also on Linux)


The protocol (from the code linked above) is currently:

// ProgCmd is a command that can be issued to a child process.
//
// If the interface needs to grow, we can add new commands or new versioned
// commands like "get2".
type ProgCmd string

const (
	cmdGet   = ProgCmd("get")
	cmdPut   = ProgCmd("put")
	cmdClose = ProgCmd("close")
)

// ProgRequest is the JSON-encoded message that's sent from cmd/go to
// the GOCACHEPROG child process over stdin. Each JSON object is on its
// own line. A ProgRequest of Type "put" with BodySize > 0 will be followed
// by a line containing a base64-encoded JSON string literal of the body.
type ProgRequest struct {
	// ID is a unique number per process across all requests.
	// It must be echoed in the ProgResponse from the child.
	ID int64

	// Command is the type of request.
	// The cmd/go tool will only send commands that were declared
	// as supported by the child.
	Command ProgCmd

	// ActionID is non-nil for get and puts.
	ActionID []byte `json:",omitempty"` // or nil if not used

	// ObjectID is set for Type "put" and "output-file".
	ObjectID []byte `json:",omitempty"` // or nil if not used

	// Body is the body for "put" requests. It's sent after the JSON object
	// as a base64-encoded JSON string when BodySize is non-zero.
	// It's sent as a separate JSON value instead of being a struct field
	// send in this JSON object so large values can be streamed in both directions.
	// The base64 string body of a ProgRequest will always be written
	// immediately after the JSON object and a newline.
	Body io.Reader `json:"-"`

	// BodySize is the number of bytes of Body. If zero, the body isn't written.
	BodySize int64 `json:",omitempty"`
}

// ProgResponse is the JSON response from the child process to cmd/go.
//
// With the exception of the first protocol message that the child writes to its
// stdout with ID==0 and KnownCommands populated, these are only sent in
// response to a ProgRequest from cmd/go.
//
// ProgResponses can be sent in any order. The ID must match the request they're
// replying to.
type ProgResponse struct {
	ID  int64  // that corresponds to ProgRequest; they can be answered out of order
	Err string `json:",omitempty"` // if non-empty, the error

	// KnownCommands is included in the first message that cache helper program
	// writes to stdout on startup (with ID==0). It includes the
	// ProgRequest.Command types that are supported by the program.
	//
	// This lets us extend the gracefully over time (adding "get2", etc), or
	// fail gracefully when needed. It also lets us verify the program
	// wants to be a cache helper.
	KnownCommands []ProgCmd `json:",omitempty"`

	// For Get requests.

	Miss      bool   `json:",omitempty"` // cache miss
	OutputID  []byte `json:",omitempty"`
	Size      int64  `json:",omitempty"`
	TimeNanos int64  `json:",omitempty"` // TODO(bradfitz): document

	// DiskPath is the absolute path on disk of the ObjectID corresponding
	// a "get" request's ActionID (on cache hit) or a "put" request's
	// provided ObjectID.
	DiskPath string `json:",omitempty"`
}
@gopherbot gopherbot added this to the Proposal milestone Apr 19, 2023
@bradfitz
Copy link
Contributor Author

bradfitz commented Apr 19, 2023

A related approved proposal is #26232 for adding a similar helper process mechanism for go get auth.

@bradfitz bradfitz changed the title proposal: cmd/go: let external processes implement the internal action/output cache proposal: cmd/go: let a child process implement the internal action/output cache Apr 19, 2023
@ianlancetaylor
Copy link
Contributor

CC @bcmills @matloob

@ianlancetaylor ianlancetaylor added the GoCommand cmd/go label Apr 19, 2023
@bcmills
Copy link
Member

bcmills commented Apr 19, 2023

See previously #42785.

@bradfitz
Copy link
Contributor Author

Right, thanks! I knew I'd seen something similar before.

Unlike that proposal, this one involves no network requests or protocol buffers or auth to figure out or changing Go caching or build semantics. Just JSON over stdin/stdout.

bradfitz added a commit to tailscale/go that referenced this issue Apr 20, 2023
Via setting GOCACHEPROG to a binary which speaks JSON over
stdin/stdout.

Updates golang#59719

Signed-off-by: Brad Fitzpatrick <bradfitz@golang.org>
Change-Id: I824ff04d5ebdf0ba4d1b5bc2e9fbaee26d34c80f
@gopherbot
Copy link

Change https://go.dev/cl/486715 mentions this issue: cmd/go: abstract build cache, support implementations via child process

bradfitz added a commit to tailscale/go that referenced this issue Apr 20, 2023
Via setting GOCACHEPROG to a binary which speaks JSON over
stdin/stdout.

Updates golang#59719

Signed-off-by: Brad Fitzpatrick <bradfitz@golang.org>
Change-Id: I824ff04d5ebdf0ba4d1b5bc2e9fbaee26d34c80f
@earthboundkid
Copy link
Contributor

JSON over stdin is probably the easiest plug-in option. The other one worth ruling out is running a WASI interpreter. WASI has the advantage that you can sandbox it and then run plug-ins you only mostly trust.

@bcmills
Copy link
Member

bcmills commented Apr 20, 2023

The main issue I foresee with JSON is that it is fairly inefficient for binary blobs, unless the idea is to use JSON to transmit filenames from the regular build cache..?

@bradfitz
Copy link
Contributor Author

@bcmills, see the implementation for details, but in summary: for Gets, only the path on disk is returned. From child process to cmd/go it's all JSON objects with no binary data. For Puts (from cmd/go to the child), the base64 binary is streamed to the client process after the JSON object metadata. So it's technically a bunch of JSON values (some objects, some strings) but can be implemented efficiently without slurping it all into memory.

@bradfitz
Copy link
Contributor Author

@carlmjohnson, if you want to run run WASI or Node or C# or a JVM you can pass a path to a program doing that. We're not going to bundle a WASI runtime into cmd/go. :)

bradfitz added a commit to tailscale/go that referenced this issue Apr 21, 2023
Via setting GOCACHEPROG to a binary which speaks JSON over
stdin/stdout.

Updates golang#59719

Signed-off-by: Brad Fitzpatrick <bradfitz@golang.org>
Change-Id: I824ff04d5ebdf0ba4d1b5bc2e9fbaee26d34c80f
bradfitz added a commit to tailscale/go that referenced this issue Apr 21, 2023
Via setting GOCACHEPROG to a binary which speaks JSON over
stdin/stdout.

Updates golang#59719

Signed-off-by: Brad Fitzpatrick <bradfitz@golang.org>
Change-Id: I824ff04d5ebdf0ba4d1b5bc2e9fbaee26d34c80f
bradfitz added a commit to tailscale/go that referenced this issue Apr 21, 2023
Via setting GOCACHEPROG to a binary which speaks JSON over
stdin/stdout.

Updates golang#59719

Signed-off-by: Brad Fitzpatrick <bradfitz@golang.org>
Change-Id: I824ff04d5ebdf0ba4d1b5bc2e9fbaee26d34c80f
bradfitz added a commit to tailscale/go that referenced this issue Apr 23, 2023
Via setting GOCACHEPROG to a binary which speaks JSON over
stdin/stdout.

Updates golang#59719

Signed-off-by: Brad Fitzpatrick <bradfitz@golang.org>
Change-Id: I824ff04d5ebdf0ba4d1b5bc2e9fbaee26d34c80f
bradfitz added a commit to tailscale/go that referenced this issue Apr 24, 2023
Via setting GOCACHEPROG to a binary which speaks JSON over
stdin/stdout.

Updates golang#59719

Change-Id: I824ff04d5ebdf0ba4d1b5bc2e9fbaee26d34c80f
@Xuanwo
Copy link

Xuanwo commented Apr 24, 2023

Great proposal! Additionally, sccache (a ccache-like compiler caching tool with native cloud storage support) is also interested in integrating with this feature.

@bradfitz
Copy link
Contributor Author

And I've posted example child process code at https://github.com/bradfitz/go-tool-cache.

@sluongng
Copy link

I would appreciate if we could get a doc in this proposal the specification of the go cache program:

  1. what are the commands
  2. what are the args to each command
  3. examples
  4. measure to ensure backward compatibility

It would help us separate between implementation and specification.


An alternative implementation I have in mind is to leverage https://github.com/bazelbuild/remote-apis/ which many build tools have started to adopt (Bazel, Buck2, Pants2, recc etc...). For example: separation between fetching Action and fetching Object might be ideal.

@Xuanwo
Copy link

Xuanwo commented Apr 27, 2023

Hello, @bradfitz. I am attempting to manually implement the Go cache protocol and have noticed that all implementations require similar concepts and logic surrounding actionID, outputID, objectID, and DiskCache.

Would it be possible to conceal these concepts within Go's built-in disk cache so that the implementation of cacher can be simplified as follows? Another benefit is that we don't have to transfer content in base64 JSON format through stdin.

For example:

// Request is the JSON-encoded message that's sent from cmd/go to
// the GOCACHEPROG child process over stdin. Each JSON object is on its
// own line. A Request of Type "put" with BodySize > 0 will be followed
// by a line containing a base64-encoded JSON string literal of the body.
type Request struct {
	// ID is a unique number per process across all requests.
	// It must be echoed in the Response from the child.
	ID int64

	// Command is the type of request.
	// The cmd/go tool will only send commands that were declared
	// as supported by the child.
	Command Cmd

	// ObjectID is set for Type "put" and "output-file".
	OutputID []byte `json:",omitempty"` // or nil if not used

        DiskPath string `json:",omitempty"`
}

// Response is the JSON response from the child process to cmd/go.
//
// With the exception of the first protocol message that the child writes to its
// stdout with ID==0 and KnownCommands populated, these are only sent in
// response to a Request from cmd/go.
//
// Responses can be sent in any order. The ID must match the request they're
// replying to.
type Response struct {
	ID  int64  // that corresponds to Request; they can be answered out of order
	Err string `json:",omitempty"` // if non-empty, the error

	// KnownCommands is included in the first message that cache helper program
	// writes to stdout on startup (with ID==0). It includes the
	// Request.Command types that are supported by the program.
	//
	// This lets us extend the gracefully over time (adding "get2", etc), or
	// fail gracefully when needed. It also lets us verify the program
	// wants to be a cache helper.
	KnownCommands []Cmd `json:",omitempty"`

	// For Get requests.
	OutputID  []byte `json:",omitempty"`

	// DiskPath is the absolute path on disk of the ObjectID corresponding
	// a "get" request's ActionID (on cache hit) or a "put" request's
	// provided ObjectID.
	DiskPath string `json:",omitempty"`
}

All cacher will just need to handle requests and implement the following API:

// Load cache of given outputID and write into diskPath
fn Get(ctx context.Context, outputID string, diskPath string) (err error)

// Read content from diskPath and store for outputID
fn Put(ctx context.Context, diskPath string, outputID string) (err error)

@bradfitz
Copy link
Contributor Author

bradfitz commented May 3, 2023

@Xuanwo, I don't want cmd/go to pick the path on disk, though. I want cacher programs to be able to return paths to FUSE filesystems back to cmd/go if they'd like.

@bradfitz
Copy link
Contributor Author

bradfitz commented May 3, 2023

I've updated the top comment with the summary of the proposed protocol from the code review.

@rsc what's the process of getting a GOEXPERIMENT at least so I can get more testing on this over a release cycle without having to carry a big patch for a long time and without getting Hyrum-locked into a particular API if there's a big mistake we didn't realize. I'd ideally love to get this into Go 1.21 (behind a compile-time GOEXPERIMENT) to test it during the next 9 months and maybe get into Go 1.22 on by default.

@rsc
Copy link
Contributor

rsc commented May 3, 2023

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@bradfitz
Copy link
Contributor Author

bradfitz commented May 4, 2023

Earlier I wrote,

I'd like to do things like hook into GitHub's native caching system at a lower level (instead of the inefficient thing people do now: untarring/tarring GOCACHE archives on every CI run, which is often slower than the CI action itself)

As an example of that:

Screenshot 2023-05-03 at 7 26 30 PM

Screenshot 2023-05-03 at 7 42 16 PM

That's two mostly wasted minutes.

@rsc
Copy link
Contributor

rsc commented May 10, 2023

Retitled proposal to be about the GOEXPERIMENT, which I think we can move to likely accept.

@rsc
Copy link
Contributor

rsc commented May 17, 2023

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@josharian
Copy link
Contributor

I’m late to the party here, but a naïve question.

It seems that the proposal calls for the cache process to re-calculate the cache key based on all of the inputs. This seems potentially error-prone, and might involve sending a very large input blob over the network.

Why not instead have the local toolchain do the cache key generation, as it does now, and have the cache process take as input only that key?

@bradfitz
Copy link
Contributor Author

@josharian, no, the child process doesn't need to recalculate the cache key. It's literally the existing internal Cache interface but sent over stdin/stdout. So cmd/go is doing all the cache key/correctness work still.

@rsc
Copy link
Contributor

rsc commented May 24, 2023

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc changed the title proposal: cmd/go: add GOEXPERIMENT=gocacheprog to let a child process implement the internal action/output cache cmd/go: add GOEXPERIMENT=gocacheprog to let a child process implement the internal action/output cache May 24, 2023
@rsc rsc modified the milestones: Proposal, Backlog May 24, 2023
@dmitshur dmitshur modified the milestones: Backlog, Go1.21 May 25, 2023
@danp
Copy link
Contributor

danp commented May 27, 2023

This is very exciting, thanks!

Sorry for comment on closed issue, but:

I set out to try it with a flow like:

  1. build tip
  2. GOEXPERIMENT=cacheprog GOCACHEPROG=whatever go build ...

Based on how GOEXPERIMENT=loopvar in #60078 will work that's what I was expecting. But it didn't seem to work.

Do I understand correctly that as-is this requires something like:

  1. GOEXPERIMENT=cacheprog build tip
  2. GOCACHEPROG=whatever go build ...

If that's the case, that's fine, but since it would require maintaining a properly-built toolchain it will probably limit how much I'm able to play with it when 1.21 comes out.

@zikaeroh
Copy link
Contributor

@danp I think you probably need to be setting GOEXPERIMENT at make.bash time, e.g. GOEXPERIMENT=cacheprog ./make.bash followed by GOCACHEPROG=whatever go build. The GOEXPERIMENT system is effectively a friendly wrapper around build tags when building the toolchain.

@danp
Copy link
Contributor

danp commented Jun 1, 2023

👍 just wanted to make sure that is the intent for this particular experiment.

@bradfitz
Copy link
Contributor Author

bradfitz commented Jun 2, 2023

Yup.

@ryancurrah
Copy link

Does this apply to Go module cache as well?

@bcmills
Copy link
Member

bcmills commented Jun 16, 2023

No, the module cache remains separate. (It wasn't cleaned or populated in the same way as the build cache to begin with.)

@ryancurrah
Copy link

ryancurrah commented Jun 17, 2023

One last question. @bradfitz do you run your tests with race detection? If it is on it would ignore the cache every time, I think, and would not use the remote cache. Or am I wrong about that? I feel like I am going to learn something new here.

@seankhliao seankhliao changed the title cmd/go: add GOEXPERIMENT=gocacheprog to let a child process implement the internal action/output cache cmd/go: add GOEXPERIMENT=cacheprog to let a child process implement the internal action/output cache Oct 8, 2023
@gopherbot
Copy link

Change https://go.dev/cl/556997 mentions this issue: cmd/go: add initial cacheprog integration tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Accepted
Development

No branches or pull requests