Conversation
This commit imports some draft optimization work intended for 'kmip-ttlv', with the goal of deprecating that crate entirely. The fast scanner and formatter defined here should provide good performance and have a simple API, that 'src/de/mod.rs' needs to be rebased onto.
The only breaking change is to 'CryptographicUsageMask', defining it via 'bitflags' instead of 'enum-flags'. The latter hasn't been updated in a while and 'bitflags' is the preferred crate for this purpose.
Also expand 'impl_ttlv_serde' with struct support.
bal-e
left a comment
bal-e
left a comment
There was a problem hiding this comment.
Thanks for converting the test cases! Once you're done, I'll investigate the unexpected failures.
There was a problem hiding this comment.
Please don't add this to the repository, in case others have conflicting settings. Something like echo "*" > .helix/.gitignore will make sure it stays ignored, without even appearing in the repository's .gitignore.
| /// The server information response field is vendor specific and thus could contain anything. | ||
| /// We don't attempt to make sense of it, but we shouldn't fail to deserialize this kind of response either. | ||
| #[test] | ||
| #[ignore = "currently failing, investigate why"] |
There was a problem hiding this comment.
I'm fairly sure I didn't add support for this in the manual deserializer. I'll check up on this.
| fn fast(ttlv_bytes: &[u8]) { | ||
| let mut scanner = kmip_protocol::ttlv::fast_scan::FastScanner::new(ttlv_bytes).unwrap(); | ||
| let req = RequestMessage::fast_scan(&mut scanner).unwrap(); | ||
|
|
||
| assert_eq!( | ||
| req.header().protocol_version(), | ||
| &request::ProtocolVersion(ProtocolVersionMajor(1), ProtocolVersionMinor(0)) | ||
| ); | ||
| } | ||
|
|
||
| fn slow(ttlv_bytes: &[u8]) { | ||
| let req: RequestMessage = kmip_ttlv::de::from_slice(ttlv_bytes).unwrap(); | ||
|
|
||
| assert_eq!( | ||
| req.header().protocol_version(), | ||
| &request::ProtocolVersion(ProtocolVersionMajor(1), ProtocolVersionMinor(0)) | ||
| ); | ||
| } |
There was a problem hiding this comment.
I just realized, Criterion provides facilities to ensure things aren't getting optimized away. If you remove the assertion and just return the parsed RequestMessage, Criterion will capture it and tell the compiler not to optimize it away.
| #[rustfmt::skip] | ||
| let use_case_request_hex = concat!( | ||
| "42007801000000904200770100000048420069010000002042006A0200000004000000010000000042006B02000000040", | ||
| // ^RequestMessage ^RequestHeader ^ProtocolVersion^ProtocolVersionMajor ^ProtocolVersionMinor | ||
| "0000000000000004200500200000004000001000000000042000D0200000004000000010000000042000F010000003842", | ||
| // ^MaximumResponseSize ^BatchCount ^BatchItem ^O | ||
| "005C050000000400000018000000004200790100000020420074050000000400000001000000004200740500000004000", | ||
| // peration ^RequestPayload ^QueryFunction::Operations ^QueryFunction::Objects | ||
| "0000200000000" | ||
| ); |
There was a problem hiding this comment.
I would recommend keeping this around. While the round-trip test is important, it doesn't guarantee that the right bytes are being produced.
| // Arya TODO: document why MaybeUninit: rationale=avoids unnecessary | ||
| // performance cost of zeroing a buffer before writing to it. | ||
| // | ||
| // Arya TODO: note the limited scope: Request deserialization but not | ||
| // response deserialization, latter wanted but former most needed for use | ||
| // by kmip2pkcs11. |
There was a problem hiding this comment.
These should probably end up as GH comments on my PR instead (but I understand this is still a draft PR)
|
Only the benchmark was supposed to be pushed, the rest is accidental and I will remove it. |
ximon18
force-pushed
the
faster-manual-deserializer-with-benchmark
branch
from
576c688 to
57a0a78
Compare
bal-e
force-pushed
the
faster-manual-deserializer
branch
2 times, most recently
from
82f122e to
468db23
Compare
2 participants
No description provided.