On Fri, 3 Nov 2023 06:06:29 GMT, Daniel Jeliński <djelin...@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
