Migrate build dimension #974
Conversation
| func itemGetter(idPattern string, displayPattern string) func([]interface{}) enum { | ||
| func isUserType(t reflect.Value) bool { | ||
| // cannot currently use build.Type_UserType to check the type need to fix that. | ||
| return t.Kind() == reflect.Float64 && t.Float() == float64(2) |
There was a problem hiding this comment.
I need to get the actual value for build.Type_UserType, but cannot import the build package as gopherjs doesn't allow importing packages that use cgo, What should we do in this instance?
There was a problem hiding this comment.
I assume the cgo code is in the device package?
You could try using the build constraint // -build js in the go file that has the import "C" line.
There was a problem hiding this comment.
After looking into this I couldn't get any build constraints to keep GopherJS from trying to build the file that imports the cgo code. One idea I had was to build the proto files into javascript as well, which would not only allow this type of value checking, but would let us use the actual type layouts rather than using text templating and json string to interface mappings to read data. This would probably be another PR though.
test/robot/scheduler/replay.go
Outdated
| Gapir: tools.Host.Gapir, | ||
| VirtualSwapChainLib: tools.Host.VirtualSwapChainLib, | ||
| VirtualSwapChainJson: tools.Host.VirtualSwapChainJson, | ||
| Package: s.pkg.Id, |
test/robot/scheduler/scheduler.go
Outdated
| for _, subj := range data.Subjects.All() { | ||
| if err := s.doTrace(ctx, subj); err != nil { | ||
| errs = append(errs, err) | ||
| if tools := s.getHostTools(ctx); tools != nil { |
There was a problem hiding this comment.
nit: Just for readability given the crazy nested depth you've now got, consider:
tools := s.getHostTools(ctx);
if tools == nil {
continue
}| func itemGetter(idPattern string, displayPattern string) func([]interface{}) enum { | ||
| func isUserType(t reflect.Value) bool { | ||
| // cannot currently use build.Type_UserType to check the type need to fix that. | ||
| return t.Kind() == reflect.Float64 && t.Float() == float64(2) |
There was a problem hiding this comment.
I assume the cgo code is in the device package?
You could try using the build constraint // -build js in the go file that has the import "C" line.
baldwinn860
force-pushed
the
migrate_build_dimension
branch
from
6065b00 to
6745d22
Compare
The robot webserver needed to have a better understanding of packages and builds. Prior to this change builds were typically looked at through the lense of the GapidAPK and Gapit versions that were used to trace. This caused multiple issues aside from just being not user-friendly. We now have a package id used with the input for each worker, and the webserver recognizes that as a separate dimension. A separate dimension is used to hold the build that was used for tracing specifically, since that feeds into the input for Report and Replay. The Host dimension now identifies the host machine that runs the worker, rather than the host machine that ran the trace which is less useful, as we already have the target as a dimension. Also need a template function mapping to test if a package type is User or not.
It is not scalable to run report and replay on every historical trace every time a new package shows up. Eventually Golden Traces will be used to run report and replay no matter the package, but for now lock the workers to only use the same package for all actions. Also remove the tracePackage dimension.
baldwinn860
force-pushed
the
migrate_build_dimension
branch
from
6745d22 to
654ba85
Compare
This change removes the gapit and gapid_apk dimensions from the grid and adds a single package dimension. There is a bit of fallout from this change, workers now must match the package that created the input (i.e. replay/report must have the same package that created the tracefile) and packages are treated as complete sets of tools rather than piecemeal subsets.