On Wed, 1 Nov 2023 21:31:02 GMT, Daniel Jeliński <djelin...@openjdk.org> wrote:

>> 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?

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.

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

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

Reply via email to