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