RFR: 8319197: Exclude hb-subset and hb-style from compilation

Magnus Ihse Bursie ihse at openjdk.org
Mon Nov 6 12:30:12 UTC 2023


On Fri, 3 Nov 2023 06:06:29 GMT, Daniel Jeliński <djelinski at openjdk.org> wrote:

>> You can find the file here:
>> https://github.com/openjdk/jdk/blob/f262f06c97b9ea94cd6119b3a8beb16bf804d083/src/java.desktop/share/native/libharfbuzz/graph/gsubgpos-context.cc
>> 
>> Hb-subset files are not guarded by ifdefs; HB_NO_SUBSET_LAYOUT only excludes some functions, but not others. I'll check the impact of that define when I'm back in office.
>> 
>> I checked the list of HB APIs [here](https://harfbuzz.github.io/reference-manual.html) ; hb-subset and hb-style belong to non-core API sets, so I decided to check if hb-style was needed. It was not.
>> 
>> We can safely stop compiling the files listed because:
>> - none of the harfbuzz functions are exported, and they are not accessible outside of libfontmanager.
>> - if we excluded any file that was actually used, linker would refuse to link libfontmanager
>> 
>> Given that we only copy an arbitrary subset of harfbuzz files anyway (see UPDATING.txt), would it make sense to remove these files instead?
>
> FWIW, I compiled the code without this PR, but with `HARFBUZZ_CFLAGS += -DHB_NO_SUBSET_LAYOUT -DHB_NO_SUBSET_CFF` instead, and checked `make LOG=profile` output. Results:
> - without this change, compiling `hb-subset.cc` took 56 seconds, and `hb-subset-plan.cc` took 33 seconds
> - with this change, compiling `hb-subset.cc` took 33 seconds, and `hb-subset-plan.cc` took 22 seconds
> 
> It's a nice improvement, but not compiling these files at all is still much better.

@djelinski Just curious; what would the effect be to both include this change and setting the NO_* defines?

If all the references to these defines are made in the excluded files then the only reason for doing that would be some kind of information to subsequent readers of the code, but they might also be checked elsewhere, and thus give an additional speedup.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16440#discussion_r1383253003


More information about the client-libs-dev mailing list