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

Daniel Jeliński djelinski at openjdk.org
Wed Nov 1 21:34:01 UTC 2023


On Wed, 1 Nov 2023 18:52:15 GMT, Phil Race <prr at openjdk.org> wrote:

>> make/modules/java.desktop/lib/Awt2dLibraries.gmk line 499:
>> 
>>> 497:    LIBFONTMANAGER_EXCLUDE_FILES += hb-ft.cc hb-subset-cff-common.cc \
>>> 498:        hb-subset-cff1.cc hb-subset-cff2.cc hb-subset-input.cc hb-subset-plan.cc \
>>> 499:        hb-subset.cc hb-subset-instancer-solver.cc gsubgpos-context.cc hb-style.cc
>> 
>> I'm quite confused from where you got gsubgpos-context.cc. I don't see any file of that name.
>> Also I just realised that since the Nov 2020 import of freetype it has a bunch of handly ifdefs that
>> you can see in hb-config.h
>> And so probably a better way of doing this, even if it means one of these files (the instance-solver one) doesn't seem to get the ifdef treatment is to use the specific defines passed to the build.
>> Then I'd expect harfbuzz to apply the same ifdef even if these files got refactored.
>> I further noticed that the same 2020 update adds HAVE_FREETYPE and hb-ft.cc isn't compiled unless you actually set that. Which we don't
>> So this whole exclude can go away and be replaced by a few -D defines.
>> You could probably go overboard and experiment with all sorts of combinations from hb-config.h but I recommend you just apply the minimal set that makes your build time happier.
>> hb-style.cc isn't one I'd have realised we don't currently need .. and for many of them it isn't clear to me without research what the consequences would be of excluding it. Without that support would something non-optimal happen in some cases - you can't trust the tests we have to come near proving it.
>> I know we don't subset, and that's the only one I consider "safe" to use here without actual research.
>
> These two
> #define HB_NO_SUBSET_LAYOUT
> #define HB_NO_SUBSET_CFF

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?

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

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


More information about the build-dev mailing list