servo / unicode-bidi

Implementation of the Unicode Bidirection Algorithm in Rust
Other
74 stars 34 forks source link

[smallvec] Use SmallVec for some internal scratch vectors #127

Closed jfkthame closed 4 months ago

jfkthame commented 4 months ago

We have some temporary vectors used during processing that will usually only hold a few entries. By using TinyVec here, we can avoid extra heap allocations in common cases.

(In Gecko's usage, I'm seeing around 4% improvement on the bidi- resolution subtest of perf_reftest_singletons with this change.)

Manishearth commented 4 months ago

This crate deliberately has zero mandatory deps: it is a transitive dependency of url which gets used by most http crates, and is at the bottom of most deptrees.

I'm fine with a quick (safe) singlepurpose enum impl of tinyvec being written here, or with making this an optional dep. I slightly prefer the former.

jfkthame commented 4 months ago

This crate deliberately has zero mandatory deps: it is a transitive dependency of url which gets used by most http crates, and is at the bottom of most deptrees.

I'm fine with a quick (safe) singlepurpose enum impl of tinyvec being written here, or with making this an optional dep. I slightly prefer the former.

Fair enough. For now I'm inclined to go with making it an optional dependency, as that seems quite a bit simpler than providing our own implementation of it, though if anyone wants to do that as a future enhancement I'd be fine with it.

I'll (force-)push a new version where it's optional. (I've also switched it from tinyvec to smallvec, which is equivalent for this purpose but better from my POV because Gecko already has it as a dependency.)