b-r-u / osmpbf

A Rust library for reading the OpenStreetMap PBF file format (*.osm.pbf).
Apache License 2.0
120 stars 18 forks source link

Use protobuf-codegen-pure to autogen proto files #12

Closed nyurik closed 2 years ago

nyurik commented 2 years ago
nyurik commented 2 years ago

P.S. I think the auto-built approach used by osmpbfreader might be even better here -- not to check in the generated files at all, but instead generate them on the fly, including the mod.rs file. And it is better to use the OUT_DIR env var instead of the hardcoded one. What do you think?

b-r-u commented 2 years ago

Thanks! I'm still hesitant about using protobuf codegen in build.rs by default, because I'm concerned about build-times, caches and the number of moving parts, but maybe I'm too conservative here?

And the solution in osmpbfreader indeed looks better!

nyurik commented 2 years ago

Thanks! I'm still hesitant about using protobuf codegen in build.rs by default, because I'm concerned about build-times, caches and the number of moving parts, but maybe I'm too conservative here?

Just to make sure, I just made it into a standalone example... and I somehow don't think real-time code building has measurable effect :)))

❯ time target/release/examples/t
real    0m0.014s
user    0m0.008s
sys 0m0.007s

Also, relying on the run-time code build is no different than relying on the dependency lib to be compile-able - we mitigate it with stricter version requirements in the Cargo.toml, and we can always switch to an older version if compilation fails. The benefits on the other hand is that we are always up to date with the protobuf lib - whatever it generates matches whatever the current protobuf runtime expects, plus we can easily experiment with changes to the .proto files themselves (I have a few ideas of how to make OSM pbf files faster to process). What do you think?

b-r-u commented 2 years ago

Alright, so the overhead of running the codegen is negligible, thanks for the measurement! Adding the protobuf-codegen-pure dependency adds 5 seconds to a 20 second clean release build for me - I guess that's okay. And after the initial build everything is cached and I couldn't measure any differences.

I also added a commit that implements writing to OUT_DIR so that the src directory is not changed by the build script.