Re: RFR [XS]: 8234795: ix build fails on macOS lower than 10.13 after 8214578 was: build fails on macOS 10.12 after 8214578: [macos] Problem with backslashes on macOS/JIS keyboard: Java ignores system
The fix is pushed in jdk/client, after pre-integration-testing (PIT) is done, it will be propagated to jdk/jdk. Regards Prasanta On 26-Nov-19 6:09 PM, Baesken, Matthias wrote: Thanks for the update on this . Do you plan to push it today or tomorrow ? Best regards, Matthias -Original Message- From: Prasanta Sadhukhan Sent: Dienstag, 26. November 2019 10:26 To: Baesken, Matthias ; Erik Joelsson ; 'build-dev@openjdk.java.net' Cc: 2d-...@openjdk.java.net; Lindenmaier, Goetz Subject: Re: RFR [XS]: 8234795: ix build fails on macOS lower than 10.13 after 8214578 was: build fails on macOS 10.12 after 8214578: [macos] Problem with backslashes on macOS/JIS keyboard: Java ignores system settings I have already raised a similar fix sometime back for JDK-8234786 Regards Prasanta On 26-Nov-19 2:49 PM, Baesken, Matthias wrote: Hello, here is a small adjustment that uses the typealias for NSTextInputSourceIdentifier . This fixes the build on macOS < 10.13 . Bug/webrev : https://bugs.openjdk.java.net/browse/JDK-8234795 http://cr.openjdk.java.net/~mbaesken/webrevs/8234795.0/ Thanks, Matthias If there is a simple fix, I would very much like to see it done. I'm not familiar enough with this area to know what the implications would be though. /Erik On 2019-11-25 04:48, Baesken, Matthias wrote: Hello, any comments on the issue ? Could we maybe switch from using NSTextInputSourceIdentifier to String (NSString* ?) , because https://developer.apple.com/documentation/appkit/nstextinputsourceiden tifier says NSTextInputSourceIdentifieris a typealias for String ? Best regards ,Matthias Hello, I noticed that since today our jdk/jdk build fails on macOS . We run it on macOS 10.12 . It seems https://hg.openjdk.java.net/jdk/jdk/rev/d0bfaae2ff33 8214578: [macos] Problem with backslashes on macOS/JIS keyboard: Java ignores system settings Brought a dependency on 10.13. Was that intended ? Could we keep 10.12 compatibility ? At least the doc of NSTextInputSourceIdentifier : https://developer.apple.com/documentation/appkit/nstextinputsourceiden tifier mentions macOS 10.13+ . Build errors are : /jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTView.h:41:5: error: unknown type name 'NSTextInputSourceIdentifier' NSTextInputSourceIdentifier kbdLayout; ^ /jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTView.m:93:23: error: assignment to readonly property self.cglLayer = windowLayer; ~ ^ ~~~ /jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTView.m:110:1 9: error: assignment to readonly property self.cglLayer = nil; ~ ^ ~~~ 3 errors generated. ... /jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.m:45 4:18: error: incompatible pointer to integer conversion initializing 'BOOL' (aka 'signed char') with an expression of type 'id' [-Werror,-Wint-conversion] BOOL mouseIsOver = [[window contentView] mouseIsOver]; ^ ~~ 2 errors generated. Best regards, Matthias
Re: RFR 8234697: Generate sun.security.util.math.intpoly classes during build
On 2019-11-26 16:39, Weijun Wang wrote: On Nov 27, 2019, at 12:14 AM, Erik Joelsson wrote: On 2019-11-25 16:42, Weijun Wang wrote: On Nov 26, 2019, at 12:36 AM, Erik Joelsson wrote: Build change looks good. Thanks. One question: I see the output to stdout from FieldGen.java shown in build. Is it possible to hide it but still store the output in the log file? No, we are not able to support different log levels to the main log file and console. What you can do is add $(LOG_INFO) or $(LOG_DEBUG) last on the command line. That macro resolves to "> /dev/null" when we don't want the output printed. How much output and how interesting is it to see? You can also wrap the whole command in a call to ExecuteWithLog to have it being logged to an individual file, which gets repeated at the end of the build in case it fails. I think you should be able to combine the two with something like this: $(call ExecuteWithLog, ...) $(LOG_DEBUG) That should print everything to the special log file as well as letting it pass to stdout when LOG=debug, but I haven't tested it. The log shows on the console with LOG=debug but whatever LOG level I choose, the output is always collected in the log file. This is exactly what I am looking for. Note that it will not show up in build.log, but in a special log file just for this command, along with a .cmdline file for this command. The location of these files need to be provided to ExecuteWithLog in the first argument, as a basename for the files. In this case it would make sense to store them next to the touch file of the rule. /Erik Thanks, Max /Erik I copied everything from CLDR_GEN_DONE but that one does not log anything actually. I can remove all output too, not really important. Thanks, Max /Erik On 2019-11-22 18:59, Weijun Wang wrote: Please review the change at http://cr.openjdk.java.net/~weijun/8234697/webrev.00/ The new lines in Gensrc-java.base.gmk mimics the one for CLDR_GEN_DONE at the beginning of the same file. I changed the BigInteger parameter in the FieldParams constructor (the one not reading terms) to its HEX string form and is now using the inverse. This makes sure the strings appeared here are exactly the same as the one used in CurveDB.java. The generated source code is identical to before. Other smaller changes made to FieldGen.jsh: 1. Package name 2. No more jshell, but now Java 3. Input/output paths as args for main() 4. White spaces, wrapping and indentation Thanks, Max
Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation
yes - The attempted incremental patch is not much use. The source files were moved several times, and despite using "hg addremove -s 60", many of the files show as a remove and a new file. /Andy On 11/26/2019 9:01 PM, Philip Race wrote: > I believe otherwise it is an accurate incremental webrev of the jpackage changes since EA-5. It is also not very incremental. * *src/jdk.incubator.jpackage/share/classes/jdk/incubator/jpackage/main/Main.java is definitely not "new" ... -phil. On 11/26/19, 2:17 PM, Andy Herrick wrote: yes, this incremental webrev contains the JNLPConverter code, which it should not. I believe otherwise it is an accurate incremental webrev of the jpackage changes since EA-5. /Andy On 11/26/2019 4:53 PM, Phil Race wrote: Andy, I am puzzled by what I see here > The webrev at [3] shows the changes since EA-06 (Build 13-jpackage+1-49 ) up to the current point. > [3] http://cr.openjdk.java.net/~herrick/8212780/webrev.06-10/ This includes the JNLPConverter which isn't what I expected to see and (correctly) isn't in the cumulative webrev Since [3] seemed like the most obvious thing to do a review of the recent changes I'd like to be sure it has the correct content and I am not sure it does ... -phil. On 11/26/19 1:36 PM, Kevin Rushforth wrote: This all looks good. +1 -- Kevin On 11/26/2019 12:56 PM, Erik Joelsson wrote: (adding build-dev) Build changes look good. /Erik On 2019-11-20 09:37, Andy Herrick wrote: I posted new composite webrev [7], and git patch [8] after pushing fix for JDK-8234402 [6]. [7] http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-11/ [8] http://cr.openjdk.java.net/~herrick/8212780/JDK-EA-11.git.patch /Andy On 11/19/2019 3:13 PM, Kevin Rushforth wrote: I took the "git diff" patch [5] that you uploaded yesterday, applied it, and verified that it is the same as what is in the JDK-8200758-branch branch of the sandbox. It builds and runs fine for me. I ran jcheck and it found the following three files with whitespace errors that will need to be fixed before you push: src/jdk.incubator.jpackage/linux/classes/jdk/incubator/jpackage/internal/PackageProperty.java:49: Trailing whitespace src/jdk.incubator.jpackage/share/classes/jdk/incubator/jpackage/ToolProviderFactory.java:35: Trailing whitespace test/jdk/tools/jpackage/share/AddLauncherBase.java:137: Trailing whitespace The second of these will go away with the fix for JDK-8234402 [6], so you don't need to do anything there. Once the fix for JDK-8234402 is pushed to sandbox, I presume you will update the webrev, right? Pending the fix for JDK-8234402 and the needed white-space fixes, this is a +1 from me (although I am not a jdk Project Reviewer, so you will need at least one review from someone who is). -- Kevin [5] http://cr.openjdk.java.net/~herrick/8212780/JDK-EA-10.git.patch [6] https://bugs.openjdk.java.net/browse/JDK-8234402 On 11/13/2019 4:23 PM, Andy Herrick wrote: Please review changes for [1] which is the implementation bug for JEP-343. The webrev at [2] is the total cumulative webrev of changes for the jpackage tool, currently in the JDK-8200758-branch branch of the open sandbox repository. The webrev at [3] shows the changes since EA-06 (Build 13-jpackage+1-49 ) up to the current point. The latest EA (Build 14-jpackage+1-49 ) is posted at [4]. Please send feedback to core-libs-...@openjdk.java.net [1] https://bugs.openjdk.java.net/browse/JDK-8212780 [2] http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-10/ [3] http://cr.openjdk.java.net/~herrick/8212780/webrev.06-10/ [4] http://jdk.java.net/jpackage/
building libjvm with -Os for space optimization - was : RE: RFR: 8234525: enable link-time section-gc for linux s390x to remove unused code
Hello Martin, I checked building libjvm.so with -Os (instead of -O3) . I used gcc-7 on linux x86_64 . The size of libjvm.so dropped from24M (normal night make with -O3) to 18M ( test make with -Os) . (adding the link-time gc might reduce the size by another ~ 10 % , but those 2 builds were without the ltgc ) Cannot say much so far about performance impact . Best regards, Matthias > > Hi Matthias and Erik, > > I also think this is an interesting option. > > I like the idea to generate smaller libraries. In addition to that, I could > also > imagine building with -Os (size optimized) by default and only select -O3 for > performance critical files (e.g. C2's register allocation, some gc code, ...). > > If we want to go into such a direction for all linux platforms and want to use > this s390 only change as some kind of pipe cleaner, I think this change is > fine > and can get pushed. > Otherwise, I think building s390 differently and not intending to do the same > for other linux platforms would be not so good. > > We should only make sure the exported symbols are set up properly to avoid > that this optimization throws out too much. > > My 50 Cents. > > Best regards, > Martin >
RFR: 8234835 Use UTF-8 charset in make support Java code
Please review this patch to make explicit use of the UTF-8 charset in build tools' IO code. JDK-8065704 changed the platform default to US-ASCII, so the intended effect of this change is to address a regression and restore the typical earlier behavior. My particular interest is in fixuppandoc, but it seems like we might has well patch all of this code to avoid relying on the platform default. http://cr.openjdk.java.net/~dlsmith/8234835/
Re: building libjvm with -Os for space optimization - was : RE: RFR: 8234525: enable link-time section-gc for linux s390x to remove unused code
Hi, we discussed doing the opposite for Mac OS X recently, where builds are currently set to -Os by default. -O3 helped various networking (micro)benchmarks by up to 20%. Rather than doing -Os by default and then cherry-pick things over to -O3 on a case-by-case basis, I'd suggest the opposite: keep -O3 as the default, start evaluating -Os on a case-by-case basis. This allows for an incremental approach where we identify things that are definitely not performance critical, e.g., never shows up in profiles, and switch those compilation units over to -Os. Check for harmful performance impact and expected footprint improvement; rinse; repeat. $.02 /Claes On 2019-11-27 17:36, Baesken, Matthias wrote: Hello Martin, I checked building libjvm.so with -Os (instead of -O3) . I used gcc-7 on linux x86_64 . The size of libjvm.so dropped from24M (normal night make with -O3) to 18M ( test make with -Os) . (adding the link-time gc might reduce the size by another ~ 10 % , but those 2 builds were without the ltgc ) Cannot say much so far about performance impact . Best regards, Matthias Hi Matthias and Erik, I also think this is an interesting option. I like the idea to generate smaller libraries. In addition to that, I could also imagine building with -Os (size optimized) by default and only select -O3 for performance critical files (e.g. C2's register allocation, some gc code, ...). If we want to go into such a direction for all linux platforms and want to use this s390 only change as some kind of pipe cleaner, I think this change is fine and can get pushed. Otherwise, I think building s390 differently and not intending to do the same for other linux platforms would be not so good. We should only make sure the exported symbols are set up properly to avoid that this optimization throws out too much. My 50 Cents. Best regards, Martin
Re: RFR: 8234835 Use UTF-8 charset in make support Java code
fixuppandoc is notable because it reads generated files .. which presumably contain non-ASCII UTF-8 characters. For the other files, it seems strange to force the use of a charset which is different from the charset of record for all our source files (i.e. US-ASCII). I'm not saying that UTF-8 isn't a good choice, but it seems questionable to be inconsistent. --Jon On 11/27/19 9:04 AM, Dan Smith wrote: Please review this patch to make explicit use of the UTF-8 charset in build tools' IO code. JDK-8065704 changed the platform default to US-ASCII, so the intended effect of this change is to address a regression and restore the typical earlier behavior. My particular interest is in fixuppandoc, but it seems like we might has well patch all of this code to avoid relying on the platform default. http://cr.openjdk.java.net/~dlsmith/8234835/
RE: building libjvm with -Os for space optimization - was : RE: RFR: 8234525: enable link-time section-gc for linux s390x to remove unused code
Hi Claes, that kind of surprises me. I'd expect files which rather benefit from -O3 to be far less than those which benefit from -Os. Most performance critical code lives inside the code cache and is not dependent on C++ compiler optimizations. I'd expect GC code, C2's register allocation and a few runtime files to be the most performance critical C++ code. So the list of files for -Os may become long. Yeah, I think we should use native profiling information to find out what's really going on. Your idea to change file by file and check for performance regression makes sense to me, though. Best regards, Martin > -Original Message- > From: Claes Redestad > Sent: Mittwoch, 27. November 2019 18:57 > To: Baesken, Matthias ; Doerr, Martin > ; Erik Joelsson ; 'build- > d...@openjdk.java.net' ; 'hotspot- > d...@openjdk.java.net' > Subject: Re: building libjvm with -Os for space optimization - was : RE: RFR: > 8234525: enable link-time section-gc for linux s390x to remove unused code > > Hi, > > we discussed doing the opposite for Mac OS X recently, where builds are > currently set to -Os by default. -O3 helped various networking > (micro)benchmarks by up to 20%. > > Rather than doing -Os by default and then cherry-pick things over to -O3 > on a case-by-case basis, I'd suggest the opposite: keep -O3 as the > default, start evaluating -Os on a case-by-case basis. This allows for > an incremental approach where we identify things that are definitely not > performance critical, e.g., never shows up in profiles, and switch those > compilation units over to -Os. Check for harmful performance impact and > expected footprint improvement; rinse; repeat. > > $.02 > > /Claes > > > On 2019-11-27 17:36, Baesken, Matthias wrote: > > Hello Martin, I checked building libjvm.so with -Os (instead of -O3) . > > > > I used gcc-7 on linux x86_64 . > > The size of libjvm.so dropped from24M (normal night make with > > -O3) > to 18M ( test make with -Os) . > > (adding the link-time gc might reduce the size by another ~ 10 % , but > those 2 builds were without the ltgc ) > > > > Cannot say much so far about performance impact . > > > > Best regards, Matthias > > > > > > > >> > >> Hi Matthias and Erik, > >> > >> I also think this is an interesting option. > >> > >> I like the idea to generate smaller libraries. In addition to that, I > >> could also > >> imagine building with -Os (size optimized) by default and only select -O3 > for > >> performance critical files (e.g. C2's register allocation, some gc code, > >> ...). > >> > >> If we want to go into such a direction for all linux platforms and want to > use > >> this s390 only change as some kind of pipe cleaner, I think this change is > fine > >> and can get pushed. > >> Otherwise, I think building s390 differently and not intending to do the > same > >> for other linux platforms would be not so good. > >> > >> We should only make sure the exported symbols are set up properly to > avoid > >> that this optimization throws out too much. > >> > >> My 50 Cents. > >> > >> Best regards, > >> Martin > >> > >
Re: RFR: 8234835 Use UTF-8 charset in make support Java code
The ubiquitous use of UTF-8 is one of the few clear successes in the software world. In 2019, most software projects should declare that all their source files are encoded in UTF-8, not US-ASCII. On Wed, Nov 27, 2019 at 9:04 AM Dan Smith wrote: > Please review this patch to make explicit use of the UTF-8 charset in > build tools' IO code. > > JDK-8065704 changed the platform default to US-ASCII, so the intended > effect of this change is to address a regression and restore the typical > earlier behavior. > > My particular interest is in fixuppandoc, but it seems like we might has > well patch all of this code to avoid relying on the platform default. > > http://cr.openjdk.java.net/~dlsmith/8234835/
Re: RFR: 8234835 Use UTF-8 charset in make support Java code
Martin, Agreed, but this is not the email-thread to have that discussion on behalf of all OpenJDK. -- Jon On 11/27/2019 10:18 AM, Martin Buchholz wrote: The ubiquitous use of UTF-8 is one of the few clear successes in the software world. In 2019, most software projects should declare that all their source files are encoded in UTF-8, not US-ASCII. On Wed, Nov 27, 2019 at 9:04 AM Dan Smith wrote: Please review this patch to make explicit use of the UTF-8 charset in build tools' IO code. JDK-8065704 changed the platform default to US-ASCII, so the intended effect of this change is to address a regression and restore the typical earlier behavior. My particular interest is in fixuppandoc, but it seems like we might has well patch all of this code to avoid relying on the platform default. http://cr.openjdk.java.net/~dlsmith/8234835/
Re: building libjvm with -Os for space optimization - was : RE: RFR: 8234525: enable link-time section-gc for linux s390x to remove unused code
Hi Martin, On 2019-11-27 19:03, Doerr, Martin wrote: Hi Claes, that kind of surprises me. I'd expect files which rather benefit from -O3 to be far less than those which benefit from -Os. Most performance critical code lives inside the code cache and is not dependent on C++ compiler optimizations. I'd expect GC code, C2's register allocation and a few runtime files to be the most performance critical C++ code. So the list of files for -Os may become long. that might very well be the end result, and once/if we've gone down this path long enough to see that -O3 becomes the exception, we can re- examine the default. Changing the default and then trying to recuperate would be hard/impossible to do incrementally. Yeah, I think we should use native profiling information to find out what's really going on > Your idea to change file by file and check for performance regression makes sense to me, though Hopefully we don't have to do one RFE per file.. :-) /Claes
Re: RFR 8234697: Generate sun.security.util.math.intpoly classes during build
> On Nov 27, 2019, at 9:44 PM, Erik Joelsson wrote: > > On 2019-11-26 16:39, Weijun Wang wrote: >> >>> On Nov 27, 2019, at 12:14 AM, Erik Joelsson >>> wrote: >>> >>> On 2019-11-25 16:42, Weijun Wang wrote: > On Nov 26, 2019, at 12:36 AM, Erik Joelsson > wrote: > > Build change looks good. Thanks. One question: I see the output to stdout from FieldGen.java shown in build. Is it possible to hide it but still store the output in the log file? >>> No, we are not able to support different log levels to the main log file >>> and console. What you can do is add $(LOG_INFO) or $(LOG_DEBUG) last on the >>> command line. That macro resolves to "> /dev/null" when we don't want the >>> output printed. How much output and how interesting is it to see? You can >>> also wrap the whole command in a call to ExecuteWithLog to have it being >>> logged to an individual file, which gets repeated at the end of the build >>> in case it fails. >>> >>> I think you should be able to combine the two with something like this: >>> >>> $(call ExecuteWithLog, ...) $(LOG_DEBUG) >>> >>> That should print everything to the special log file as well as letting it >>> pass to stdout when LOG=debug, but I haven't tested it. >> The log shows on the console with LOG=debug but whatever LOG level I choose, >> the output is always collected in the log file. This is exactly what I am >> looking for. > > Note that it will not show up in build.log, but in a special log file just > for this command, Yes, that's what I meant. > along with a .cmdline file for this command. The location of these files need > to be provided to ExecuteWithLog in the first argument, as a basename for the > files. In this case it would make sense to store them next to the touch file > of the rule. Yes, that's what I copied from CLDR_GEN_DONE, the argument, and the touch. It does not have "$(LOG_DEBUG)" at the end of the call but it has no output anyway. The special log file is empty. Thanks, Max > > /Erik > >> Thanks, >> Max >> >>> /Erik >>> I copied everything from CLDR_GEN_DONE but that one does not log anything actually. I can remove all output too, not really important. Thanks, Max > /Erik > > On 2019-11-22 18:59, Weijun Wang wrote: >> Please review the change at >> >>http://cr.openjdk.java.net/~weijun/8234697/webrev.00/ >> >> The new lines in Gensrc-java.base.gmk mimics the one for CLDR_GEN_DONE >> at the beginning of the same file. >> >> I changed the BigInteger parameter in the FieldParams constructor (the >> one not reading terms) to its HEX string form and is now using the >> inverse. This makes sure the strings appeared here are exactly the same >> as the one used in CurveDB.java. The generated source code is identical >> to before. >> >> Other smaller changes made to FieldGen.jsh: >> >> 1. Package name >> 2. No more jshell, but now Java >> 3. Input/output paths as args for main() >> 4. White spaces, wrapping and indentation >> >> Thanks, >> Max
Re: RFR: 8234835 Use UTF-8 charset in make support Java code
> For the other files, it seems strange to force the use of a charset > which is different from the charset of record for all our source files > (i.e. US-ASCII). Can you clarify where this "charset of record" rule comes from? Is this written down somewhere, or more of an oral tradition? The non-ASCII characters I'm working with are, in fact, in the original Markdown sources. If it's really important to avoid those in all sources, I could (reluctantly) use a different strategy. If the consensus is that the build tools should standardize on US-ASCII, I guess there's a separate question about whether we're willing to rely on the implicit platform default (now uniformly US-ASCII via command-line args), or whether it's better to be explicit about it (s/UTF_8/US_ASCII/ in my changeset).
