protovm: add compiler#5784
Conversation
🎨 Perfetto UI Builds
|
rukkal
force-pushed
the
dev/rukkal/protovm-compiler
branch
4 times, most recently
from
496f16a to
c458f69
Compare
Introduces a compiler for ProtoVM that translates high-level configuration commands into low-level bytecode instructions (VmProgram). CompileConfig proto: - Defines a sequence of high-level commands CLI: - Reads CompileConfig textproto from stdin and outputs the compiled binary VmProgram to stdout. Internal Compiler and InstructionEmitter classes: - Parse compile configs (textproto) - Resolve human-readable paths (field names) into field IDs using TracePacket and WinscopeExtensions descriptors. - Convert high-level commands into the corresponding sequence of VmInstruction. Change-Id: Icc63a5e653726f1b94100138099a80a9f99562a4
rukkal
force-pushed
the
dev/rukkal/protovm-compiler
branch
from
c458f69 to
808c987
Compare
primiano
left a comment
primiano
left a comment
There was a problem hiding this comment.
THe code makes sense, my main suggestion is:
-
use protoc_lib to pass the .proto file from cmdline rather than baking assumptions on which protos/descriptors you are going to need (or find a way to bundle them all, not just the winscope descriptor)
-
Some more comments would help for future reading
the rest looks great
| syntax = "proto2"; | ||
|
|
||
| package perfetto.protos; | ||
|
|
There was a problem hiding this comment.
add some comments to explain what this is about?
| namespace perfetto::protovm { | ||
|
|
||
| Compiler::Compiler() : emitter_(&pool_) { | ||
| pool_.AddFromFileDescriptorSet(perfetto::kTraceDescriptor.data(), |
There was a problem hiding this comment.
rather than building the descriptors in the tool, I think it woudl be more flebible if we depend on //gn:protoc_lib and we allow passing .proto files from the cmdline.
This makes the compiler more flexible, because you can just point it to the proto you need, rather than assuming "I'm going to need winscope so i am going to bundle them inside the tool"
|
|
||
| namespace perfetto::protovm { | ||
|
|
||
| // Implements the compiler front-end. |
There was a problem hiding this comment.
can we have README.md (or a /docs/design-doc) that explains what this compiler does. Just ask AI to turn your doc + code here into a README.md for here.
| public: | ||
| Compiler(); | ||
|
|
||
| base::StatusOr<std::string> Compile(std::string_view config_textproto); |
There was a problem hiding this comment.
add a comment saying which message config_textproto is supposed to match
| syntax = "proto2"; | ||
|
|
||
| package perfetto.protos; | ||
|
|
There was a problem hiding this comment.
can you add a comment / explain what these protos are for? and how they are different from vm_program.proto?
| const protos::pbzero::Command::Decoder& command) const; | ||
|
|
||
| perfetto::trace_processor::DescriptorPool pool_; | ||
| const perfetto::trace_processor::ProtoDescriptor* root_proto_; |
| #include "perfetto/ext/base/file_utils.h" | ||
| #include "src/protovm/compiler/compiler.h" | ||
|
|
||
| int ProcessArgs(int argc, char** argv); |
There was a problem hiding this comment.
put this in the anon namespace
| } | ||
|
|
||
| perfetto_host_executable("protovm_compiler") { | ||
| sources = [ "main.cc" ] |
There was a problem hiding this comment.
if you make it testonly=true you can depend on protoc lib
Change-Id: Ibd7ece2b5c3dd218e8549494913ed68cc8c92493
2 participants
protovm: add compiler
Introduces a compiler for ProtoVM that translates high-level configuration commands into
low-level bytecode instructions (VmProgram).
CompileConfig proto:
CLI:
to stdout.
Internal Compiler and InstructionEmitter classes:
WinscopeExtensions descriptors.