Evolve CollationSpecialPrimariesV1 data struct#7872
Evolve CollationSpecialPrimariesV1 data struct#7872robertbastian wants to merge 4 commits intounicode-org:mainfrom
CollationSpecialPrimariesV1 data struct#7872Conversation
robertbastian
requested review from
a team,
Manishearth,
echeran,
hsivonen and
sffc
as code owners
robertbastian
force-pushed
the
primaries
branch
from
bce48e0 to
afbcd33
Compare
sffc
left a comment
sffc
left a comment
There was a problem hiding this comment.
I like how the code is structured better with this PR.
robertbastian
force-pushed
the
primaries
branch
from
afbcd33 to
9c245ed
Compare
robertbastian
force-pushed
the
primaries
branch
from
9c245ed to
ecef237
Compare
| let jamo = jamo.unwrap(); | ||
|
|
||
| impl<'a> CollatorBorrowed<'a> { | ||
| fn collation_elements<C: Iterator<Item = char>>( |
There was a problem hiding this comment.
Did you re-run benchmarks to see that this doesn't regress perfomance? From the code change, it looks like it shouldn't, but there have been surprises here previously.
There was a problem hiding this comment.
Specifically, we need to make sure that rustc keeps initializing CollationElementsdirectly into its eventual memory location instead of initializing it first and then moving it.
There was a problem hiding this comment.
Well, the ::new call is a way more complex function than this, so if anything doesn't get inlined it's that (and that's preexisting). But I can run benchmarks to confirm, is it just cargo bench?
There was a problem hiding this comment.
cargo criterion, rather, with measures to make the clock stable (CPU governor set to performance and Intel turbo or AMD boost turned off). I even do setarch -R cargo criterion just in case, but I'm not sure if that bit matters.
(And, yes, it looks OK, but there's previously been a regression here that looked OK from source.)
There was a problem hiding this comment.
ok I've inlined this instead. this is now code-equivalent to the previous macro version, with the upside that we have types and stuff
3 participants
Now that we can change the baked representation, this evolves the data struct while keeping the serialized representation stable, removing the hacks we had added for this.
Changelog
icu_collator: Changes to theCollationSpecialPrimariesV1data struct