-
Notifications
You must be signed in to change notification settings - Fork 2
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
perf: make fair benchmark comparison #30
base: main
Are you sure you want to change the base?
Conversation
The premise for your BMs is that a chunk needs to be converted to a vector in every case. So the benchmarks showcase that for almost all instances chunks are slower apart from prepend. This contradicts the whole purpose of having this data structure when in reality we are combating the performance penalty we need to deal with while working with vecs. In the real world, you won't always materialize the value as a vector.
Overall, I don't think the premise that Chunks have to convert to Vec to do anything meaningful is inaccurate. |
That's the point of this pr - showcase the example close to the real use case, otherwise the benchmarks could be treated as manually crafted to showcase the benefits that are unlikely achievable. Let's break the examples you've provided:
The current benchmark feels more like comparing iterator adapters e.g. |
Concat comparison currently is fair. Typically in any immutable DS, concat guarantees that if two instances are concatenated, the internal order of values of the resulting DS follows the order of concatenation. It doesn't expect that each value should be accessible by index and considering its immutable, it needs to ensure that none of the two values are modified. The proposed benchmarks don't perform concatenation technically in the vector implementation. It modifies the base value with the new value which is not a fair comparison because it's causing a mutation. The same is true for append. Appending a value only needs to guarantee that while iterating it should be the last value produced. Again the vector implementation isn't immutable and hence not an apples-to-apples comparison. |
The previous benchmark implementation was focused only on the called operations itself ignoring the part of deferred calculation that actually should be done to be able to use the calculated result.
Why is it important? To make the comparison more fair since when using Vec and its methods we get the entity that could be used immediately for acquiring the specific values or iterating over. But for
Chunk
we need to call.as_vec
method before we can use similar things.It changes the results significantly and could represent downsides of using
Chunk
more clearly.Here are the results on my machine: