typst / ecow

Compact, clone-on-write vector and string.
Apache License 2.0
208 stars 16 forks source link

`<EcoVec as Extend>::extend` does extra work #38

Closed kaikalii closed 6 months ago

kaikalii commented 8 months ago

I've noticed that the implementation of <EcoVec as Extend>::extend does a lot of extra work. It calls EcoVec::push in a loop. Each push call does a bunch of different checks in EcoVec::reserve.

I started implementing a PR to use either Iterator::size_hint or ExactSizeIterator along with EcoVec::extend_from_trusted, but I learned from their docs that neither should be relied upon for maintaining unsafe invariants.

One thought I had was to simply expose EcoVec::extend_from_trusted and/or EcoVec::push_unchecked as pub. Another idea would be to simply add an EcoVec::extend_from_vec and/or EcoVec::extend_from_ecovec, whose lengths and iterator counts are known to be always consistent.

Would either of these options be appropriate to implement? I'd like to be able to get the maximum performance possible from EcoVecs.

laurmaedje commented 8 months ago

The special function extend_from_slice already exists, but there is precedent for that in std. For push_unchecked and extend_from_trusted, there is no precedent there (although extend of course has the specialization in std).

In smallvec, I found this which should at least be a bit more efficient. Maybe you could research what other vec libraries do?

Beyond optimizing extend at least a bit with the size hint, from the options suggested above extend_from_trusted sounds best to me.

laurmaedje commented 6 months ago

Solved by https://github.com/typst/ecow/pull/39