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

Corrections to prost benchmark #62

Merged
merged 3 commits into from
Feb 6, 2024
Merged

Conversation

mumbleskates
Copy link
Contributor

The prost benchmark is currently discarding the Log::address field, causing it to appear to have significantly higher performance and better compactness than it should.

Before:

log/prost/deserialize                                       1.00  1558.1±16.12µs
log/prost/serialize (encode)                                1.00    235.3±7.06µs
log/prost/serialize (populate + encode)                     1.00   1076.9±3.38µs

log/prost/size 764951
log/prost/zlib 268137
log/prost/zstd 227947

After:

log/prost/deserialize                                       1.00  1941.7±22.57µs
log/prost/serialize (encode)                                1.00    465.8±3.36µs
log/prost/serialize (populate + encode)                     1.00  1248.0±17.10µs

log/prost/size 884628
log/prost/zlib 363130
log/prost/zstd 315494

I also added guardrails to the other benches as well, but this is the main one that it appeared to be performing far better than possible. It is successfully round-tripping values in the other benches.

@djkoloski
Copy link
Owner

Looks like there's a new issue with nightlies related to aHash 0.7. I'll try to update bson-rust to the latest version since that would eliminate the old dependency.

mumbleskates and others added 3 commits February 6, 2024 12:50
… of the "logs" bench

More serializers should be forced to check this! the data they serialized should be fully recoverable!
this significantly slows its encoding in this bench and increases its encoded/compressed size by 15/38%
@djkoloski djkoloski merged commit 2a8527e into djkoloski:master Feb 6, 2024
1 check passed
@mumbleskates
Copy link
Contributor Author

I can't get an unrestricted set of features to work no matter what i install, flatbuffer generated files are totally wrong with the tool i get in ubuntu 22.04 & 23.10. that's a separate problem though

@djkoloski
Copy link
Owner

What issues are you seeing? Are they related to the generated Rust files?

@mumbleskates
Copy link
Contributor Author

mumbleskates commented Feb 6, 2024

i get at least 11 errors like

error[E0405]: cannot find trait `SafeSliceAccess` in crate `flatbuffers`
  --> src/datasets/log/log_generated.rs:42:19
   |
42 | impl flatbuffers::SafeSliceAccess for Address {}
   |                   ^^^^^^^^^^^^^^^ not found in `flatbuffers`

probable issue:

$ flatc --version
flatc version 2.0.8

...but it's kind of a big workflow pain to need to go get and install the latest one to run this, especially considering there are already generated files checked in that it just wants to stomp.

the repo won't build until i install both capnp and flatc but all those seem to do is mess up their respective already-checked-in code.

@djkoloski
Copy link
Owner

Yeah that's been a problem for a while. As an immediate-term fix you could just disable the build.rs code generation. But for a longer-term fix, I think we'd want to pull down the latest versions of flatc/protoc/capnp from their respective GitHub repos. I already do this manually for Windows because I have no choice.

@mumbleskates
Copy link
Contributor Author

here's a mini-kludge for it that should work: #63

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants