3ab36fda9a1788e0 introduced a ~3% regression on some decoding
benchmarks. My best guess is that swapping the value temporarily with a
new string is not being optimized well. This commit reverts to using
String::as_mut_vec, but carefully clears the string on decode failure.
It also fixes up the associated regression test, which wasn't properly
covering the malformed UTF-8 case.
// Before #194
dataset/google_message3_2/decode
time: [219.45 ms 219.48 ms 219.51 ms]
thrpt: [458.64 MiB/s 458.72 MiB/s 458.78 MiB/s]
// #194
dataset/google_message3_2/decode
time: [226.08 ms 226.11 ms 226.18 ms]
thrpt: [445.12 MiB/s 445.25 MiB/s 445.32 MiB/s]
// This commit
dataset/google_message3_2/decode
time: [219.05 ms 219.76 ms 221.78 ms]
thrpt: [453.94 MiB/s 458.12 MiB/s 459.61 MiB/s]
They were inhibiting rustdoc from generating code for nested impl
blocks, and are no longer necessary since the new 2018 edition fully
qualified imports.
* Strengthen some inlines and remove a cast
Signed-off-by: Nick Cameron <nrc@ncameron.org>
* Optimise bytes::merge by pre-allocating and using an efficient copy
Signed-off-by: Nick Cameron <nrc@ncameron.org>
* Use get_unchecked rather than indexing where it is safe to do so
Signed-off-by: Nick Cameron <nrc@ncameron.org>
* Use `Bytes::put` rather than our memcpy in `bytes::merge`
Authored by Dan Burkert <dan@danburkert.com>
* Add a recursion-limit feature
Signed-off-by: Nick Cameron <nrc@ncameron.org>
* Thread a DecodeContext through decoding
Signed-off-by: Nick Cameron <nrc@ncameron.org>
* Add recursion limit checks
Signed-off-by: Nick Cameron <nrc@ncameron.org>
* Test for recursion limit
Signed-off-by: Nick Cameron <nrc@ncameron.org>
* Document the recursion limit
Signed-off-by: Nick Cameron <nrc@ncameron.org>
* wip
Signed-off-by: Nick Cameron <nrc@ncameron.org>
* Use a 'by value' implementation of DecodeContext
Signed-off-by: Nick Cameron <nrc@ncameron.org>
* Reviewer changes
Primarily changing the `recursion-limit` feature to `no-recursion-limit`,
Signed-off-by: Nick Cameron <nrc@ncameron.org>
* Move the DecodeContext code to keep the varint code together
Signed-off-by: Nick Cameron <nrc@ncameron.org>
* Add tests and fix docs
Signed-off-by: Nick Cameron <nrc@ncameron.org>
* Address reviewer comments
No breaking changes to the API, attributes formatted consistently.
Signed-off-by: Nick Cameron <nrc@ncameron.org>
* Apply suggestions from code review
Co-Authored-By: Dan Burkert <dan@danburkert.com>
* Merge string value without as_mut_vec unsoundness
In case of an encoding error in string::merge, the appended string value
is left with broken UTF-8 in case of an error. The same can happen if
any of the Buf methods panics. This results in UB if the string value is used
after the error return or in unwind, respectively.
Change the implementation to truncate the string back to valid UTF-8
content in any code path that does not go through validation of the
newly appended bytes.
* Unit test for string::merge failure
Test that adding an invalid UTF-8 sequence results in an error,
and that the string is reverted to its state prior to the merge call.
* Safe reimplementation of string::merge
As suggested by Dan Burkert:
https://github.com/danburkert/prost/pull/194#discussion_r292096009
The existing string value is dropped in case of a merge failure,
but this is better than exposing an invalid value.
