On Wed, 1 Nov 2023 18:51:26 GMT, Phil Race <p...@openjdk.org> wrote:

>> hb-subset and hb-style APIs are not used and not exported by libfontmanger. 
>> We can cut the compilation time by not compiling the unused files.
>> 
>> The added exclusions reduce the build time by ~1 minute (~8%) on my machine. 
>> This is with harfbuzz 8, with harfbuzz 7 the improvement was ~30 seconds.
>> 
>> Client libs tests continue to pass.
>
> 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

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

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

Reply via email to