Re: RFR: 8283323: libharfbuzz optimization level results in extreme build times [v3]
On Wed, 23 Mar 2022 23:38:52 GMT, Sergey Bylokhov wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Update comment to show we don't care about these files. > > Do we really want to change the product performance characteristics based on > the compilation time? Can we try parallelize/cache or something first? > @mrserb Phil said that we don't even use the subset part of harfbuzz. > > And unless you're backing up your claim that this patch is changing runtime > characteristic with data, that's just a guess, just like the initial > optimization level of this library (and as most of the native libraries in > the JDK... :-() was just a guess. Otoh, that we get a build time regression > for these two files is a proven fact. > > What is it you want to cache? It is a reasonable question. Build times might seem important to a small number of developers but runtime performance is King. That's why I asked to reduce it to just the files we don't care about. Would we give up even 0.1% of hotspot GC performance on Linux to avoid 30 seconds of compile time ? I doubt it. And haven't we improved compile time a lot with the parallelism ? So are we now over-optimising in the wrong place ? I mean good to keep an eye on it but I mean if a previous build arch took 15 mins and now with parallelism we take 3 mins and 45 secs .. is it so bad to be back up to 4 mins for more runtime benefit ? (Numbers entirely made up) There's always going to be a long pole (something that is last) and the questions are whether its understood and for a good reason and so forth. I mean we can doubtless try to improve but at some point it is diminishing returns. Perhaps someone can ask gcc devs why they are so slow on this code ? Maybe there's a good reason that helps runtime or maybe they just have design issues. They may want to know. - PR: https://git.openjdk.java.net/jdk/pull/7919
Re: RFR: 8283323: libharfbuzz optimization level results in extreme build times [v3]
On Wed, 23 Mar 2022 23:38:52 GMT, Sergey Bylokhov wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Update comment to show we don't care about these files. > > Do we really want to change the product performance characteristics based on > the compilation time? Can we try parallelize/cache or something first? @mrserb Phil said that we don't even use the subset part of harfbuzz. And unless you're backing up your claim that this patch is changing runtime characteristic with data, that's just a guess, just like the initial optimization level of this library (and as most of the native libraries in the JDK... :-() was just a guess. Otoh, that we get a build time regression for these two files is a proven fact. What is it you want to cache? - PR: https://git.openjdk.java.net/jdk/pull/7919
Re: RFR: 8283323: libharfbuzz optimization level results in extreme build times [v3]
On Wed, 23 Mar 2022 23:17:38 GMT, Magnus Ihse Bursie wrote: >> [JDK-8247872](https://bugs.openjdk.java.net/browse/JDK-8247872) (upgrade >> HarfBuzz to 2.7.2) caused build time to go up with 24 seconds on my >> reference linux machine. This was one of the four culprits that caused a >> 25-30% build time regression over the last two years. >> >> The problem here was that the new HarfBuzz code caught really bad behaviour >> from gcc when compiling with optimizations. The official HarfBuzz build does >> not use any -O flags at all for gcc, so presumably the HarfBuzz team is: >> >> a) not thinking compiler optimization is important for the performance of >> this library, and >> b) unaware that their code causes such a headache for gcc. >> >> (Other compilers fare much better: visual studio makes no difference at all, >> and for clang just a small regression was observed.) >> >> The current optimization level was introduced by >> [JDK-8255790](https://bugs.openjdk.java.net/browse/JDK-8255790), which were >> really about moving libharfbuzz compilation back into libfontmanager. I >> could find no comments/discussion relating to the change of optimization >> level, so I assume it was incidental, and just seemed good at the time. >> >> This patch changes the optimization level to `SIZE` (which is the closest >> thing we have to no optimization level) on gcc. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Update comment to show we don't care about these files. Do we really want to change the product performance characteristics based on the compilation time? Can we try parallelize/cache or something first? - PR: https://git.openjdk.java.net/jdk/pull/7919
Re: RFR: 8283323: libharfbuzz optimization level results in extreme build times [v2]
On Wed, 23 Mar 2022 23:13:04 GMT, Magnus Ihse Bursie wrote: >> make/modules/java.desktop/lib/Awt2dLibraries.gmk line 487: >> >>> 485: LIBFONTMANAGER_OPTIMIZATION := HIGHEST >>> 486: >>> 487: ifneq ($(filter $(TOOLCHAIN_TYPE), gcc clang), ) >> >> Can we have a note here that the de-opt is possible because these two files >> aren't important to OpenJDK ? >> If these files were to be renamed in an updated version of harfbuzz, would >> there be a compilation error ? > > Comment updated. > > No, these directives will be just silently ignored. So if that happens we run > the risk of reintroducing the build performance regression. Not a big risk. > In any case, at next harfbuzz update it can be worth re-checking if these > exceptions are still needed. (If you're the one doing it and need help with > that, just let me know!) As of today they are still there in upstream harfbuzz on github. - PR: https://git.openjdk.java.net/jdk/pull/7919
Re: RFR: 8283323: libharfbuzz optimization level results in extreme build times [v3]
On Wed, 23 Mar 2022 23:17:38 GMT, Magnus Ihse Bursie wrote: >> [JDK-8247872](https://bugs.openjdk.java.net/browse/JDK-8247872) (upgrade >> HarfBuzz to 2.7.2) caused build time to go up with 24 seconds on my >> reference linux machine. This was one of the four culprits that caused a >> 25-30% build time regression over the last two years. >> >> The problem here was that the new HarfBuzz code caught really bad behaviour >> from gcc when compiling with optimizations. The official HarfBuzz build does >> not use any -O flags at all for gcc, so presumably the HarfBuzz team is: >> >> a) not thinking compiler optimization is important for the performance of >> this library, and >> b) unaware that their code causes such a headache for gcc. >> >> (Other compilers fare much better: visual studio makes no difference at all, >> and for clang just a small regression was observed.) >> >> The current optimization level was introduced by >> [JDK-8255790](https://bugs.openjdk.java.net/browse/JDK-8255790), which were >> really about moving libharfbuzz compilation back into libfontmanager. I >> could find no comments/discussion relating to the change of optimization >> level, so I assume it was incidental, and just seemed good at the time. >> >> This patch changes the optimization level to `SIZE` (which is the closest >> thing we have to no optimization level) on gcc. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Update comment to show we don't care about these files. Marked as reviewed by prr (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7919
Re: RFR: 8283323: libharfbuzz optimization level results in extreme build times [v3]
> [JDK-8247872](https://bugs.openjdk.java.net/browse/JDK-8247872) (upgrade > HarfBuzz to 2.7.2) caused build time to go up with 24 seconds on my reference > linux machine. This was one of the four culprits that caused a 25-30% build > time regression over the last two years. > > The problem here was that the new HarfBuzz code caught really bad behaviour > from gcc when compiling with optimizations. The official HarfBuzz build does > not use any -O flags at all for gcc, so presumably the HarfBuzz team is: > > a) not thinking compiler optimization is important for the performance of > this library, and > b) unaware that their code causes such a headache for gcc. > > (Other compilers fare much better: visual studio makes no difference at all, > and for clang just a small regression was observed.) > > The current optimization level was introduced by > [JDK-8255790](https://bugs.openjdk.java.net/browse/JDK-8255790), which were > really about moving libharfbuzz compilation back into libfontmanager. I could > find no comments/discussion relating to the change of optimization level, so > I assume it was incidental, and just seemed good at the time. > > This patch changes the optimization level to `SIZE` (which is the closest > thing we have to no optimization level) on gcc. Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Update comment to show we don't care about these files. - Changes: - all: https://git.openjdk.java.net/jdk/pull/7919/files - new: https://git.openjdk.java.net/jdk/pull/7919/files/b491f6fb..99642af3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7919&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7919&range=01-02 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7919.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7919/head:pull/7919 PR: https://git.openjdk.java.net/jdk/pull/7919
Re: RFR: 8283323: libharfbuzz optimization level results in extreme build times [v2]
On Wed, 23 Mar 2022 22:37:55 GMT, Phil Race wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Restore HIGHEST for entire lib, just set SIZE to two files > > make/modules/java.desktop/lib/Awt2dLibraries.gmk line 487: > >> 485: LIBFONTMANAGER_OPTIMIZATION := HIGHEST >> 486: >> 487: ifneq ($(filter $(TOOLCHAIN_TYPE), gcc clang), ) > > Can we have a note here that the de-opt is possible because these two files > aren't important to OpenJDK ? > If these files were to be renamed in an updated version of harfbuzz, would > there be a compilation error ? Comment updated. No, these directives will be just silently ignored. So if that happens we run the risk of reintroducing the build performance regression. Not a big risk. In any case, at next harfbuzz update it can be worth re-checking if these exceptions are still needed. (If you're the one doing it and need help with that, just let me know!) - PR: https://git.openjdk.java.net/jdk/pull/7919
Re: RFR: 8283323: libharfbuzz optimization level results in extreme build times [v2]
On Wed, 23 Mar 2022 21:13:30 GMT, Magnus Ihse Bursie wrote: >> [JDK-8247872](https://bugs.openjdk.java.net/browse/JDK-8247872) (upgrade >> HarfBuzz to 2.7.2) caused build time to go up with 24 seconds on my >> reference linux machine. This was one of the four culprits that caused a >> 25-30% build time regression over the last two years. >> >> The problem here was that the new HarfBuzz code caught really bad behaviour >> from gcc when compiling with optimizations. The official HarfBuzz build does >> not use any -O flags at all for gcc, so presumably the HarfBuzz team is: >> >> a) not thinking compiler optimization is important for the performance of >> this library, and >> b) unaware that their code causes such a headache for gcc. >> >> (Other compilers fare much better: visual studio makes no difference at all, >> and for clang just a small regression was observed.) >> >> The current optimization level was introduced by >> [JDK-8255790](https://bugs.openjdk.java.net/browse/JDK-8255790), which were >> really about moving libharfbuzz compilation back into libfontmanager. I >> could find no comments/discussion relating to the change of optimization >> level, so I assume it was incidental, and just seemed good at the time. >> >> This patch changes the optimization level to `SIZE` (which is the closest >> thing we have to no optimization level) on gcc. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Restore HIGHEST for entire lib, just set SIZE to two files make/modules/java.desktop/lib/Awt2dLibraries.gmk line 487: > 485: LIBFONTMANAGER_OPTIMIZATION := HIGHEST > 486: > 487: ifneq ($(filter $(TOOLCHAIN_TYPE), gcc clang), ) Can we have a note here that the de-opt is possible because these two files aren't important to OpenJDK ? If these files were to be renamed in an updated version of harfbuzz, would there be a compilation error ? - PR: https://git.openjdk.java.net/jdk/pull/7919
Re: RFR: 8283575: Check for GNU time fails for version >1.7 [v2]
> The version output of GNU time changed from "GNU time" to "GNU Time" in > version 1.8. We need to update our check for identifying GNU time to handle > this. Erik Joelsson has updated the pull request incrementally with one additional commit since the last revision: Update make/autoconf/basic_tools.m4 Co-authored-by: Magnus Ihse Bursie - Changes: - all: https://git.openjdk.java.net/jdk/pull/7925/files - new: https://git.openjdk.java.net/jdk/pull/7925/files/2cf533f0..04d6bf49 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7925&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7925&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7925.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7925/head:pull/7925 PR: https://git.openjdk.java.net/jdk/pull/7925
Re: RFR: 8283323: libharfbuzz optimization level results in extreme build times [v2]
On Wed, 23 Mar 2022 21:13:30 GMT, Magnus Ihse Bursie wrote: >> [JDK-8247872](https://bugs.openjdk.java.net/browse/JDK-8247872) (upgrade >> HarfBuzz to 2.7.2) caused build time to go up with 24 seconds on my >> reference linux machine. This was one of the four culprits that caused a >> 25-30% build time regression over the last two years. >> >> The problem here was that the new HarfBuzz code caught really bad behaviour >> from gcc when compiling with optimizations. The official HarfBuzz build does >> not use any -O flags at all for gcc, so presumably the HarfBuzz team is: >> >> a) not thinking compiler optimization is important for the performance of >> this library, and >> b) unaware that their code causes such a headache for gcc. >> >> (Other compilers fare much better: visual studio makes no difference at all, >> and for clang just a small regression was observed.) >> >> The current optimization level was introduced by >> [JDK-8255790](https://bugs.openjdk.java.net/browse/JDK-8255790), which were >> really about moving libharfbuzz compilation back into libfontmanager. I >> could find no comments/discussion relating to the change of optimization >> level, so I assume it was incidental, and just seemed good at the time. >> >> This patch changes the optimization level to `SIZE` (which is the closest >> thing we have to no optimization level) on gcc. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Restore HIGHEST for entire lib, just set SIZE to two files Changing just these two files removes most of the build time regression. I'm okay with that. Going forward I'd like to understand *why* this is on the critical path, and if there's anything we can do about it. But that's much more complex than just addressing this regression in build times. I'm also pondering if we could help make somehow by hinting that files like subset and layout take a long time to compile, and schedule them early, so we don't suddenly find us inn the position of waiting for just them to be able to link the library. I'm suspecting that if the file was named hb-aarrdwark.cc it would not have impacted the total build time as much as now... - PR: https://git.openjdk.java.net/jdk/pull/7919
Re: RFR: 8283575: Check for GNU time fails for version >1.7
On Wed, 23 Mar 2022 15:35:44 GMT, Erik Joelsson wrote: > The version output of GNU time changed from "GNU time" to "GNU Time" in > version 1.8. We need to update our check for identifying GNU time to handle > this. I usually handle cases like this by prefixing with a comment: # Additional [] needed to keep m4 from mangling shell constructs. and then wrap the entire statement in an outer `[ ]` so the inner, "real" brackets are unchanged. make/autoconf/basic_tools.m4 line 351: > 349: UTIL_LOOKUP_PROGS(PATCH, gpatch patch) > 350: # Check if it's GNU time > 351: IS_GNU_TIME=`$TIME --version 2>&1 | $GREP 'GNU [[Tt]]ime'` Suggestion: [ IS_GNU_TIME=`$TIME --version 2>&1 | $GREP 'GNU [Tt]ime'` ] Like this. - PR: https://git.openjdk.java.net/jdk/pull/7925
Re: RFR: 8283323: libharfbuzz optimization level results in extreme build times [v2]
> [JDK-8247872](https://bugs.openjdk.java.net/browse/JDK-8247872) (upgrade > HarfBuzz to 2.7.2) caused build time to go up with 24 seconds on my reference > linux machine. This was one of the four culprits that caused a 25-30% build > time regression over the last two years. > > The problem here was that the new HarfBuzz code caught really bad behaviour > from gcc when compiling with optimizations. The official HarfBuzz build does > not use any -O flags at all for gcc, so presumably the HarfBuzz team is: > > a) not thinking compiler optimization is important for the performance of > this library, and > b) unaware that their code causes such a headache for gcc. > > (Other compilers fare much better: visual studio makes no difference at all, > and for clang just a small regression was observed.) > > The current optimization level was introduced by > [JDK-8255790](https://bugs.openjdk.java.net/browse/JDK-8255790), which were > really about moving libharfbuzz compilation back into libfontmanager. I could > find no comments/discussion relating to the change of optimization level, so > I assume it was incidental, and just seemed good at the time. > > This patch changes the optimization level to `SIZE` (which is the closest > thing we have to no optimization level) on gcc. Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Restore HIGHEST for entire lib, just set SIZE to two files - Changes: - all: https://git.openjdk.java.net/jdk/pull/7919/files - new: https://git.openjdk.java.net/jdk/pull/7919/files/20ab768f..b491f6fb Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7919&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7919&range=00-01 Stats: 8 lines in 1 file changed: 1 ins; 2 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/7919.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7919/head:pull/7919 PR: https://git.openjdk.java.net/jdk/pull/7919
Re: RFR: 8283323: libharfbuzz optimization level results in extreme build times
On Wed, 23 Mar 2022 12:25:08 GMT, Magnus Ihse Bursie wrote: > [JDK-8247872](https://bugs.openjdk.java.net/browse/JDK-8247872) (upgrade > HarfBuzz to 2.7.2) caused build time to go up with 24 seconds on my reference > linux machine. This was one of the four culprits that caused a 25-30% build > time regression over the last two years. > > The problem here was that the new HarfBuzz code caught really bad behaviour > from gcc when compiling with optimizations. The official HarfBuzz build does > not use any -O flags at all for gcc, so presumably the HarfBuzz team is: > > a) not thinking compiler optimization is important for the performance of > this library, and > b) unaware that their code causes such a headache for gcc. > > (Other compilers fare much better: visual studio makes no difference at all, > and for clang just a small regression was observed.) > > The current optimization level was introduced by > [JDK-8255790](https://bugs.openjdk.java.net/browse/JDK-8255790), which were > really about moving libharfbuzz compilation back into libfontmanager. I could > find no comments/discussion relating to the change of optimization level, so > I assume it was incidental, and just seemed good at the time. > > This patch changes the optimization level to `SIZE` (which is the closest > thing we have to no optimization level) on gcc. harfbuzz does not make it easy to subset the build. So no. - PR: https://git.openjdk.java.net/jdk/pull/7919
Re: RFR: 8283323: libharfbuzz optimization level results in extreme build times
On Wed, 23 Mar 2022 19:35:22 GMT, Phil Race wrote: >> [JDK-8247872](https://bugs.openjdk.java.net/browse/JDK-8247872) (upgrade >> HarfBuzz to 2.7.2) caused build time to go up with 24 seconds on my >> reference linux machine. This was one of the four culprits that caused a >> 25-30% build time regression over the last two years. >> >> The problem here was that the new HarfBuzz code caught really bad behaviour >> from gcc when compiling with optimizations. The official HarfBuzz build does >> not use any -O flags at all for gcc, so presumably the HarfBuzz team is: >> >> a) not thinking compiler optimization is important for the performance of >> this library, and >> b) unaware that their code causes such a headache for gcc. >> >> (Other compilers fare much better: visual studio makes no difference at all, >> and for clang just a small regression was observed.) >> >> The current optimization level was introduced by >> [JDK-8255790](https://bugs.openjdk.java.net/browse/JDK-8255790), which were >> really about moving libharfbuzz compilation back into libfontmanager. I >> could find no comments/discussion relating to the change of optimization >> level, so I assume it was incidental, and just seemed good at the time. >> >> This patch changes the optimization level to `SIZE` (which is the closest >> thing we have to no optimization level) on gcc. > > We definitely do not want to downgrade the optimisation level of the entire > library. > Performance of this library actually matters quite a lot. > When we added some additional checking to the previous library we used (ICU) > we had complaints of as much as 20% loss > in PDF conversion (a backend process) and we've had escalations on user > responsiveness in editors due to the computational complexity of layout vs > just counting glyph advances. > Getting that performance back was a lot of work .. except increasing compiler > optimisation was the easy one that made a big difference. > To see the performance degradation may require a specific combination of font > (Linux fonts vary widely) and the text being laid out. > The example we used in the previous case required a windows font, so I don't > have a handy test case I know will show this .. and it was years ago .. > > Looking at the list of files from Erik, I think we don't use the subset > functionality. Certainly not directly and likely not at all. > So downgrading that probably won't have any impact at all at runtime. > > But hb-ot-layout.cc is one of the most important files in the library, maybe > the most important. > > I strongly suggest that one not be downgraded just for 7 seconds of build > time. @prrace If we're not using the subset functionality, can we remove the files entirely, or exclude them from compilation? (In the meantime, I'm preparing a patch which only limits optimization to the two subset files Erik listed) - PR: https://git.openjdk.java.net/jdk/pull/7919
Re: RFR: 8283323: libharfbuzz optimization level results in extreme build times
On Wed, 23 Mar 2022 12:25:08 GMT, Magnus Ihse Bursie wrote: > [JDK-8247872](https://bugs.openjdk.java.net/browse/JDK-8247872) (upgrade > HarfBuzz to 2.7.2) caused build time to go up with 24 seconds on my reference > linux machine. This was one of the four culprits that caused a 25-30% build > time regression over the last two years. > > The problem here was that the new HarfBuzz code caught really bad behaviour > from gcc when compiling with optimizations. The official HarfBuzz build does > not use any -O flags at all for gcc, so presumably the HarfBuzz team is: > > a) not thinking compiler optimization is important for the performance of > this library, and > b) unaware that their code causes such a headache for gcc. > > (Other compilers fare much better: visual studio makes no difference at all, > and for clang just a small regression was observed.) > > The current optimization level was introduced by > [JDK-8255790](https://bugs.openjdk.java.net/browse/JDK-8255790), which were > really about moving libharfbuzz compilation back into libfontmanager. I could > find no comments/discussion relating to the change of optimization level, so > I assume it was incidental, and just seemed good at the time. > > This patch changes the optimization level to `SIZE` (which is the closest > thing we have to no optimization level) on gcc. We definitely do not want to downgrade the optimisation level of the entire library. Performance of this library actually matters quite a lot. When we added some additional checking to the previous library we used (ICU) we had complaints of as much as 20% loss in PDF conversion (a backend process) and we've had escalations on user responsiveness in editors due to the computational complexity of layout vs just counting glyph advances. Getting that performance back was a lot of work .. except increasing compiler optimisation was the easy one that made a big difference. To see the performance degradation may require a specific combination of font (Linux fonts vary widely) and the text being laid out. The example we used in the previous case required a windows font, so I don't have a handy test case I know will show this .. and it was years ago .. Looking at the list of files from Erik, I think we don't use the subset functionality. Certainly not directly and likely not at all. So downgrading that probably won't have any impact at all at runtime. But hb-ot-layout.cc is one of the most important files in the library, maybe the most important. I strongly suggest that one not be downgraded just for 7 seconds of build time. - PR: https://git.openjdk.java.net/jdk/pull/7919
Re: RFR: 8283323: libharfbuzz optimization level results in extreme build times
On Wed, 23 Mar 2022 12:25:08 GMT, Magnus Ihse Bursie wrote: > [JDK-8247872](https://bugs.openjdk.java.net/browse/JDK-8247872) (upgrade > HarfBuzz to 2.7.2) caused build time to go up with 24 seconds on my reference > linux machine. This was one of the four culprits that caused a 25-30% build > time regression over the last two years. > > The problem here was that the new HarfBuzz code caught really bad behaviour > from gcc when compiling with optimizations. The official HarfBuzz build does > not use any -O flags at all for gcc, so presumably the HarfBuzz team is: > > a) not thinking compiler optimization is important for the performance of > this library, and > b) unaware that their code causes such a headache for gcc. > > (Other compilers fare much better: visual studio makes no difference at all, > and for clang just a small regression was observed.) > > The current optimization level was introduced by > [JDK-8255790](https://bugs.openjdk.java.net/browse/JDK-8255790), which were > really about moving libharfbuzz compilation back into libfontmanager. I could > find no comments/discussion relating to the change of optimization level, so > I assume it was incidental, and just seemed good at the time. > > This patch changes the optimization level to `SIZE` (which is the closest > thing we have to no optimization level) on gcc. > I have a question, though, because I suppose font rendering engine is > performance sensitive. AFAICS, the `LIBFONTMANAGER_OPTIMIZATION` was changed > from `HIGH` to `HIGHEST` here: > [05fe06a6#diff-c75e2d581b3730cf900075cedb66ac72eaba40fbbd92eaac5710161830b572b4L534-R491](https://github.com/openjdk/jdk/commit/05fe06a6#diff-c75e2d581b3730cf900075cedb66ac72eaba40fbbd92eaac5710161830b572b4L534-R491) > > Does it mean we should instead downgrade to `HIGH`, not `SIZE`? Would that > recover the build times? For GCC on Linux, that doesn't make a difference as both HIGH and HIGHEST translate to -O3. I've been looking a bit at this too, and there are a handful of files that stick out, hb-subset.cc is the worst one. Here are the top three on my Linux host (using 31 threads), first with -O3 and then with Os: [TIME:0:18.36] hb-subset-plan.cc [TIME:0:21.53] hb-ot-layout.cc [TIME:0:35.63] hb-subset.cc [TIME:0:12.51] hb-subset-plan.cc [TIME:0:14.81] hb-ot-layout.cc [TIME:0:21.40] hb-subset.cc On my mac (6 threads, but single core perf is higher), the corresponding numbers are: [TIME:0:14.46] hb-subset-plan.cc [TIME:0:19.64] hb-ot-layout.cc [TIME:0:27.45] hb-subset.cc [TIME:0:11.69] hb-subset-plan.cc [TIME:0:16.00] hb-ot-layout.cc [TIME:0:20.68] hb-subset.cc This is certainly not quite as bad, but still a clear difference. The reason this has such a high impact on total compilation time is due to how java.desktop ends up being one of the last top level targets on the critical path of possible concurrency. So a single compilation unit here will stall the whole process on a machine with many threads (32 in my case). On a smaller machine, this impact will be much less pronounced. Here are the total compilation times for "make exploded-image" with a few different settings: all libfontmanager HIGHEST linux: 4m14.174s macosx: 7m8.736s all libfontmanager SIZE linux: 3m56.134s macosx: 7m5.966s just the above 3 files SIZE linux: 3m57.009s macosx: (skipped, no point) The difference on the smaller macosx machine is negligible. Most of the compilation time gain on the big machine is gained by just dropping the optimization level on a few files. One could certainly argue that dropping the optimization level for libfontmanager is only helping a rather niche usecase. There could certainly be other compilation units that would have a similar effect on total build time due to how they end up on the critical path, I would be surprised if there aren't. The question is which ones we can safely drop the optimization level for without adversely affect performance of the finished product. For this particular change it's probably better to at least just drop it for one or a few files. - PR: https://git.openjdk.java.net/jdk/pull/7919
Re: RFR: 8283323: libharfbuzz optimization level results in extreme build times
On Wed, 23 Mar 2022 12:25:08 GMT, Magnus Ihse Bursie wrote: > [JDK-8247872](https://bugs.openjdk.java.net/browse/JDK-8247872) (upgrade > HarfBuzz to 2.7.2) caused build time to go up with 24 seconds on my reference > linux machine. This was one of the four culprits that caused a 25-30% build > time regression over the last two years. > > The problem here was that the new HarfBuzz code caught really bad behaviour > from gcc when compiling with optimizations. The official HarfBuzz build does > not use any -O flags at all for gcc, so presumably the HarfBuzz team is: > > a) not thinking compiler optimization is important for the performance of > this library, and > b) unaware that their code causes such a headache for gcc. > > (Other compilers fare much better: visual studio makes no difference at all, > and for clang just a small regression was observed.) > > The current optimization level was introduced by > [JDK-8255790](https://bugs.openjdk.java.net/browse/JDK-8255790), which were > really about moving libharfbuzz compilation back into libfontmanager. I could > find no comments/discussion relating to the change of optimization level, so > I assume it was incidental, and just seemed good at the time. > > This patch changes the optimization level to `SIZE` (which is the closest > thing we have to no optimization level) on gcc. No, unfortunately it won't. :( The problem started with the update to HarfBuzz 2.7.2, and there the opt level was HIGH. I also tested manually compiling with -O1 (which we don't really have a level for in the build system) and even that was too problematic for gcc. But another approach is possible. I talked off-list with Erik, and he had noticed (as had I) that a single file in harfbuzz was responsible for the lion's share of the build time regression. What he did, though, that I didn't come to think of, was to try to change optimization level for just that file. This fixes most of the build time regression, while being a much less intrusive fix. It also makes sense to do this fix for clang, since the 8 second regression seen there is basically also completely due to this file. So I'll upload an update too this fix that is more conservative, and only changes opt level for a single file. - PR: https://git.openjdk.java.net/jdk/pull/7919
Re: RFR: 8283575: Check for GNU time fails for version >1.7
On Wed, 23 Mar 2022 16:22:55 GMT, Aleksey Shipilev wrote: > Looks okay, provided you tested with some version of GNU [Tt]ime. Thanks! I hit this while trying to use GNU time on my mac to get LOG=profile to work, to profile compilation times of individual compilation units. I built latest (1.9) from source and it didn't detect it (same with 1.8). With this patch it works as expected. > Is double square bracket some sort of escaping? Haven't seen it before. Yes, it's escaping needed to use brackets in autoconf/m4. Sometimes we put the escape sequence around the complete expression, sometimes we just double up like I did here. You should be able to find several occurrences through out our autoconf scripts. - PR: https://git.openjdk.java.net/jdk/pull/7925
Re: RFR: 8283575: Check for GNU time fails for version >1.7
On Wed, 23 Mar 2022 15:35:44 GMT, Erik Joelsson wrote: > The version output of GNU time changed from "GNU time" to "GNU Time" in > version 1.8. We need to update our check for identifying GNU time to handle > this. Looks okay, provided you tested with some version of GNU [Tt]ime. Is double square bracket some sort of escaping? Haven't seen it before. - Marked as reviewed by shade (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7925
Re: RFR: 8283323: libharfbuzz optimization level results in extreme build times
On Wed, 23 Mar 2022 12:25:08 GMT, Magnus Ihse Bursie wrote: > [JDK-8247872](https://bugs.openjdk.java.net/browse/JDK-8247872) (upgrade > HarfBuzz to 2.7.2) caused build time to go up with 24 seconds on my reference > linux machine. This was one of the four culprits that caused a 25-30% build > time regression over the last two years. > > The problem here was that the new HarfBuzz code caught really bad behaviour > from gcc when compiling with optimizations. The official HarfBuzz build does > not use any -O flags at all for gcc, so presumably the HarfBuzz team is: > > a) not thinking compiler optimization is important for the performance of > this library, and > b) unaware that their code causes such a headache for gcc. > > (Other compilers fare much better: visual studio makes no difference at all, > and for clang just a small regression was observed.) > > The current optimization level was introduced by > [JDK-8255790](https://bugs.openjdk.java.net/browse/JDK-8255790), which were > really about moving libharfbuzz compilation back into libfontmanager. I could > find no comments/discussion relating to the change of optimization level, so > I assume it was incidental, and just seemed good at the time. > > This patch changes the optimization level to `SIZE` (which is the closest > thing we have to no optimization level) on gcc. I have a question, though, because I suppose font rendering engine is performance sensitive. AFAICS, the `LIBFONTMANAGER_OPTIMIZATION` was changed from `HIGH` to `HIGHEST` here: https://github.com/openjdk/jdk/commit/05fe06a6#diff-c75e2d581b3730cf900075cedb66ac72eaba40fbbd92eaac5710161830b572b4L534-R491 Does it mean we should instead downgrade to `HIGH`, not `SIZE`? Would that recover the build times? - PR: https://git.openjdk.java.net/jdk/pull/7919
RFR: 8283575: Check for GNU time fails for version >1.7
The version output of GNU time changed from "GNU time" to "GNU Time" in version 1.8. We need to update our check for identifying GNU time to handle this. - Commit messages: - JDK-8283575 Changes: https://git.openjdk.java.net/jdk/pull/7925/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7925&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8283575 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7925.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7925/head:pull/7925 PR: https://git.openjdk.java.net/jdk/pull/7925
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v5]
> This PR contains the API and implementation changes for JEP-424 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/424 Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Drop redundant javadoc statements re. handling of nulls (handling of nulls is specified once and for all in the package javadoc) - Changes: - all: https://git.openjdk.java.net/jdk/pull/7888/files - new: https://git.openjdk.java.net/jdk/pull/7888/files/7ec71f73..c9bc9a70 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7888&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7888&range=03-04 Stats: 12 lines in 2 files changed: 3 ins; 8 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7888.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7888/head:pull/7888 PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8283323: libharfbuzz optimization level results in extreme build times
On Wed, 23 Mar 2022 12:25:08 GMT, Magnus Ihse Bursie wrote: > [JDK-8247872](https://bugs.openjdk.java.net/browse/JDK-8247872) (upgrade > HarfBuzz to 2.7.2) caused build time to go up with 24 seconds on my reference > linux machine. This was one of the four culprits that caused a 25-30% build > time regression over the last two years. > > The problem here was that the new HarfBuzz code caught really bad behaviour > from gcc when compiling with optimizations. The official HarfBuzz build does > not use any -O flags at all for gcc, so presumably the HarfBuzz team is: > > a) not thinking compiler optimization is important for the performance of > this library, and > b) unaware that their code causes such a headache for gcc. > > (Other compilers fare much better: visual studio makes no difference at all, > and for clang just a small regression was observed.) > > The current optimization level was introduced by > [JDK-8255790](https://bugs.openjdk.java.net/browse/JDK-8255790), which were > really about moving libharfbuzz compilation back into libfontmanager. I could > find no comments/discussion relating to the change of optimization level, so > I assume it was incidental, and just seemed good at the time. > > This patch changes the optimization level to `SIZE` (which is the closest > thing we have to no optimization level) on gcc. Marked as reviewed by erikj (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7919
RFR: 8283323: libharfbuzz optimization level results in extreme build times
[JDK-8247872](https://bugs.openjdk.java.net/browse/JDK-8247872) (upgrade HarfBuzz to 2.7.2) caused build time to go up with 24 seconds on my reference linux machine. This was one of the four culprits that caused a 25-30% build time regression over the last two years. The problem here was that the new HarfBuzz code caught really bad behaviour from gcc when compiling with optimizations. The official HarfBuzz build does not use any -O flags at all for gcc, so presumably the HarfBuzz team is: a) not thinking compiler optimization is important for the performance of this library, and b) unaware that their code causes such a headache for gcc. (Other compilers fare much better: visual studio makes no difference at all, and for clang just a small regression was observed.) The current optimization level was introduced by [JDK-8255790](https://bugs.openjdk.java.net/browse/JDK-8255790), which were really about moving libharfbuzz compilation back into libfontmanager. I could find no comments/discussion relating to the change of optimization level, so I assume it was incidental, and just seemed good at the time. This patch changes the optimization level to `SIZE` (which is the closest thing we have to no optimization level) on gcc. - Commit messages: - 8283323: libharfbuzz optimization level results in extreme build times Changes: https://git.openjdk.java.net/jdk/pull/7919/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7919&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8283323 Stats: 8 lines in 1 file changed: 7 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7919.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7919/head:pull/7919 PR: https://git.openjdk.java.net/jdk/pull/7919
Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v4]
On Wed, 23 Mar 2022 02:03:26 GMT, Fei Yang wrote:
>> This PR implements JEP 422: Linux/RISC-V Port [1].
>> The PR starts as a squashed merge of the
>> https://openjdk.java.net/projects/riscv-port branch.
>>
>> This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive
>> Unmatched board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are
>> also carried out regularly. So it should be good enough to run most Java
>> programs.
>>
>> [1] https://openjdk.java.net/jeps/422
>
> Fei Yang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fix copyright header
Looks okay to me.
-
Marked as reviewed by shade (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/6294
