Skip to content

Conversation

@cmyr
Copy link
Member

@cmyr cmyr commented Jan 16, 2026

And use it in codegen. This is intended as a proof of concept to explore performance options for harfrust.

This is similar to the first couple commits of #1709 except this is more narrowly scoped (only focused on the two hot-path methods) and is in theory intended to be mergeable if we decide to go this route.

And use it in codegen. This is intended as a proof of concept to explore
performance options for harfrust.
@cmyr cmyr force-pushed the read-unsafe-unwrap branch from 0184b92 to 0d2cdd6 Compare January 20, 2026 15:25
@behdad
Copy link
Contributor

behdad commented Jan 20, 2026

Thanks. I see the following speedup in HB's benchmark-shape:

$ subprojects/benchmark-1.8.4/tools/compare.py benchmarks before after
Comparing before to after
Benchmark                                                                               Time             CPU      Time Old      Time New       CPU Old       CPU New
--------------------------------------------------------------------------------------------------------------------------------------------------------------------
BM_Shape/Roboto-Regular.ttf/en-thelittleprince.txt/harfrust                          -0.0068         -0.0068             7             7             7             7
BM_Shape/Roboto-Regular.ttf/en-words.txt/harfrust                                    -0.0028         -0.0025            12            12            12            12
BM_Shape/SourceSerifVariable-Roman.ttf/react-dom.txt/harfrust                        -0.0110         -0.0108            78            77            78            77
BM_Shape/NotoSansDevanagari-Regular.ttf/hi-words.txt/harfrust                        -0.0052         -0.0047            27            27            27            27
BM_Shape/Amiri-Regular.ttf/fa-thelittleprince.txt/harfrust                           -0.0699         -0.0703            40            37            39            37
BM_Shape/NotoNastaliqUrdu-Regular.ttf/fa-thelittleprince.txt/harfrust                -0.1735         -0.1737            86            71            86            71
BM_Shape/NotoNastaliqUrdu-Regular.ttf/fa-words.txt/harfrust                          -0.1464         -0.1471           103            88           103            88
BM_Shape/Gulzar-Regular.ttf/fa-thelittleprince.txt/harfrust                          -0.2451         -0.2451          2203          1663          2194          1656
BM_Shape/Gulzar-Regular.ttf/fa-words.txt/harfrust                                    -0.2382         -0.2380          1426          1086          1419          1081
BM_Shape/NotoSansDuployan-Regular.otf/duployan.txt/harfrust                          -0.1990         -0.1991          2269          1817          2259          1809
OVERALL_GEOMEAN                                                                      -0.1151         -0.1151             0             0             0             0

@dfrg
Copy link
Member

dfrg commented Jan 20, 2026

There's also #1588 which might complement this and avoid generating a significant amount of unsafe code.

@cmyr
Copy link
Member Author

cmyr commented Jan 20, 2026

Yea, we are going to ultimately need to decide if we're okay with the unsafe or not. If we are then this approach is probably preferable since it will save the overhead of both storing the extra pointer as well as doing the various bits of validation required to do a bytemuck cast of the struct? But if we decide we aren't okay with the unsafe than an approach like #1588 makes more sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants