Re: RFR: 8264322: Generate CDS archive when creating custom JDK image [v2]
On Thu, 19 Aug 2021 23:59:32 GMT, Calvin Cheung wrote:
>> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/CDSPlugin.java
>> line 121:
>>
>>> 119: @Override
>>> 120: public ResourcePool transform(ResourcePool in, ResourcePoolBuilder
>>> out) {
>>> 121: in.transformAndCopy((file) -> {
>>
>> This method should not be called in a post-processor.Plugin API needs
>> some re-thinking to support post-processor plugin. As `Plugin::transform`
>> is abstract method, for now this method should simply throw
>> `UnsupportedOperationException`.
>
> It is being called. The minimal implementation is:
>
>
> @Override
> public ResourcePool transform(ResourcePool in, ResourcePoolBuilder out) {
> return in;
> }
>
> I've filed [JDK-8272734](https://bugs.openjdk.java.net/browse/JDK-8272743) to
> follow-up the issue.
Thanks for filing the issue. I didn't expect `Plugin::transform` should be
called. This is a good workaround for now.
>> test/jdk/tools/jlink/plugins/CDSPluginTest.java line 80:
>>
>>> 78:String jlinkPath = JDKToolFinder.getJDKTool("jlink");
>>> 79:String[] cmd = {jlinkPath, "--add-modules",
>>> "java.base,java.logging",
>>> 80:"-J-Dos.name=windows",
>>> "--generate-cds-archive",
>>
>> Is there a better way of setting `os.name` system property on the command
>> line?
>>
>> Maybe override `ModuleTarget` attribute in module-info.class of java.base to
>> be a different platform?
>
> I'd prefer to leave it as is for now since it's in a test case and I don't
> think the code is complex.
This is fragile by setting `os.name`.
-
PR: https://git.openjdk.java.net/jdk/pull/5174
Re: RFR: 8264322: Generate CDS archive when creating custom JDK image [v2]
On Fri, 20 Aug 2021 00:09:01 GMT, Calvin Cheung wrote: >> Please review this change for adding a `jlink` command line option >> `--generate-cds-archive` for generating CDS archives as a post processing >> step during the creation of a custom JDK image. >> >> Details can be found in the corresponding CSR >> [JDK-8269178](https://bugs.openjdk.java.net/browse/JDK-8269178). >> >> Testing: >> >> - [x] tiers 1,2 (including the new test) > > Calvin Cheung has updated the pull request incrementally with one additional > commit since the last revision: > > @mlchung comments Thanks for the change. Looks okay. - PR: https://git.openjdk.java.net/jdk/pull/5174
Re: RFR: 8264322: Generate CDS archive when creating custom JDK image [v2]
On Thu, 19 Aug 2021 23:56:00 GMT, Calvin Cheung wrote: >> src/jdk.jlink/share/classes/jdk/tools/jlink/builder/DefaultImageBuilder.java >> line 89: >> >>> 87: private final List args; >>> 88: private final Set modules; >>> 89: private Platform platform; >> >> Can `DefaultExecutableImage` constructor take an additional `platform` >> argument and make this `platform` field final? >> >> When the `DefaultExecutableImage` is constructed, it already has the target >> platform information. >> >> In the constructor, it can check if the `platform` parameter must not be >> `UNKNOWN`; otherwise throw IAE. > > I've added the `platform` argument and made the `platform` field final. > However, as we've discussed offline, there's a code path via the > `--post-process-path` option where the platform may not be available. So we > can't throw IAE on an `UNKNOWN` platform in the constructor. Since `--post-process-path` is a hidden option, it's fine not to support it. - PR: https://git.openjdk.java.net/jdk/pull/5174
Re: RFR: 8264322: Generate CDS archive when creating custom JDK image [v2]
> Please review this change for adding a `jlink` command line option > `--generate-cds-archive` for generating CDS archives as a post processing > step during the creation of a custom JDK image. > > Details can be found in the corresponding CSR > [JDK-8269178](https://bugs.openjdk.java.net/browse/JDK-8269178). > > Testing: > > - [x] tiers 1,2 (including the new test) Calvin Cheung has updated the pull request incrementally with one additional commit since the last revision: @mlchung comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/5174/files - new: https://git.openjdk.java.net/jdk/pull/5174/files/918d5066..b7a5ec68 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5174&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5174&range=00-01 Stats: 74 lines in 10 files changed: 14 ins; 39 del; 21 mod Patch: https://git.openjdk.java.net/jdk/pull/5174.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5174/head:pull/5174 PR: https://git.openjdk.java.net/jdk/pull/5174
Re: RFR: 8264322: Generate CDS archive when creating custom JDK image [v2]
On Wed, 18 Aug 2021 23:55:25 GMT, Mandy Chung wrote:
>> Calvin Cheung has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> @mlchung comments
>
> src/jdk.jlink/share/classes/jdk/tools/jlink/builder/DefaultImageBuilder.java
> line 89:
>
>> 87: private final List args;
>> 88: private final Set modules;
>> 89: private Platform platform;
>
> Can `DefaultExecutableImage` constructor take an additional `platform`
> argument and make this `platform` field final?
>
> When the `DefaultExecutableImage` is constructed, it already has the target
> platform information.
>
> In the constructor, it can check if the `platform` parameter must not be
> `UNKNOWN`; otherwise throw IAE.
I've added the `platform` argument and made the `platform` field final.
However, as we've discussed offline, there's a code path via the
`--post-process-path` option where the platform may not be available. So we
can't throw IAE on an `UNKNOWN` platform in the constructor.
> src/jdk.jlink/share/classes/jdk/tools/jlink/builder/DefaultImageBuilder.java
> line 134:
>
>> 132:
>> 133: @Override
>> 134: public void setTargetPlatform(Platform p) {
>
> This setter is not needed if it's passed to the constructor.
Fixed.
> src/jdk.jlink/share/classes/jdk/tools/jlink/builder/ImageBuilder.java line 82:
>
>> 80: * Gets the target image platform.
>> 81: *
>> 82: * @return Platform
>
> Suggestion:
>
> * @return {@code Platform} object representing the platform of the
> runtime image
Fixed.
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ExecutableImage.java
> line 70:
>
>> 68: * @param p
>> 69: */
>> 70: public void setTargetPlatform(Platform p);
>
> This method should not be needed. The platform should be known when an
> executable image is created.
Fixed.
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ExecutableImage.java
> line 75:
>
>> 73: * The Platform of the image.
>> 74: *
>> 75: * @return Platform
>
> Suggestion:
>
> * @return {@code Platform} object representing the platform of the
> runtime image
Done.
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ExecutableImage.java
> line 82:
>
>> 80: * Checks if the image is 64-bit.
>> 81: *
>> 82: * @return boolean
>
> Suggestion:
>
> * @return true if it's a 64-bit platform
Fixed.
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ExecutableImage.java
> line 84:
>
>> 82: * @return boolean
>> 83: */
>> 84: public boolean is64Bit();
>
> This `is64Bit` method should belong to `Platform` class
Done. I've moved the method to the `Platform` class.
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImagePluginStack.java
> line 195:
>
>> 193: public void operate(ImageProvider provider) throws Exception {
>> 194: ExecutableImage img = provider.retrieve(this);
>> 195: img.setTargetPlatform(imageBuilder.getTargetPlatform());
>
> See above comment - the ExecutableImage should know the platform when it's
> constructed.
Removed the code.
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Platform.java line 55:
>
>> 53: * Returns the {@code Platform} based on the platformString of the
>> form -.
>> 54: */
>> 55: public static Platform parseTargetPlatform(String platformString) {
>
> This method can be renamed back to `parsePlatform` (an earlier version).
> My suggestion to name this method as `parseTargetPlatform` no longer apply in
> this version.
Changed the name back to `parsePlatform`.
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/CDSPlugin.java
> line 80:
>
>> 78: status = p.waitFor();
>> 79: } catch (Exception ex) {
>> 80: ex.printStackTrace();
>
> This plugin should fail with any exception. jlink will handle
> `PluginException` and `UncheckedIOException` and print the error message.
> You can simply wrap IOException with `UncheckedIOException`
>
>
> try {
>Process p = builder.inheritIO().start();
>status = p.waitFor();
> } catch (IOException e) {
>throw new UncheckedIOException(e);
> }
I've changed it to the following since we also need to catch the
`InterruptedException`:
try {
Process p = builder.inheritIO().start();
status = p.waitFor();
} catch (InterruptedException | IOException e) {
throw new PluginException(e);
}
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/CDSPlugin.java
> line 83:
>
>> 81: }
>> 82: if (status == 0) {
>> 83: System.out.println("Created " + archiveMsg + " archive
>> successfully");
>
> Is it significant to print two lines on 64-bit platform? Users may not have
> any idea what `NOCOOPS` means?
>
> Created CDS archive successfully
> Created NOCOOPS CDS archive successfully
>
> It seems adeq
Re: RFR: 8266936: Add a finalization JFR event [v6]
> Greetings, > > Object.finalize() was deprecated in JDK9. There is an ongoing effort to > replace and mitigate Object.finalize() uses in the JDK libraries; please see > https://bugs.openjdk.java.net/browse/JDK-8253568 for more information. > > We also like to assist users in replacing and mitigating uses in non-JDK code. > > Hence, this changeset adds a periodic JFR event to help identify which > classes are overriding Object.finalize(). > > Thanks > Markus Markus Grönlund has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. - Changes: - all: https://git.openjdk.java.net/jdk/pull/4731/files - new: https://git.openjdk.java.net/jdk/pull/4731/files/c8dbca54..d8b829e8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4731&range=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4731&range=04-05 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4731.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731 PR: https://git.openjdk.java.net/jdk/pull/4731
Re: RFR: 8266936: Add a finalization JFR event [v5]
> Greetings, > > Object.finalize() was deprecated in JDK9. There is an ongoing effort to > replace and mitigate Object.finalize() uses in the JDK libraries; please see > https://bugs.openjdk.java.net/browse/JDK-8253568 for more information. > > We also like to assist users in replacing and mitigating uses in non-JDK code. > > Hence, this changeset adds a periodic JFR event to help identify which > classes are overriding Object.finalize(). > > Thanks > Markus Markus Grönlund has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 20 additional commits since the last revision: - Merge branches '8266936-alt' and '8266936-alt' of github.com:mgronlun/jdk into 8266936-alt - remove build directive - Code Source attribute for Finalizer event - whitespace - update - update - whitespace - 8266936-alt: Add a finalization JFR event - shortcut calling Jfr::is_recording() - 8266936: Add a finalization JFR event - ... and 10 more: https://git.openjdk.java.net/jdk/compare/663b1a74...c8dbca54 - Changes: - all: https://git.openjdk.java.net/jdk/pull/4731/files - new: https://git.openjdk.java.net/jdk/pull/4731/files/44988036..c8dbca54 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4731&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4731&range=03-04 Stats: 65693 lines in 1278 files changed: 54237 ins; 5619 del; 5837 mod Patch: https://git.openjdk.java.net/jdk/pull/4731.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731 PR: https://git.openjdk.java.net/jdk/pull/4731
Re: RFR: JDK-8272639: jpackaged applications using microphone on mac
On Thu, 19 Aug 2021 14:12:00 GMT, Andy Herrick wrote: > JDK-8272639: jpackaged applications using microphone on mac src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/Info-lite.plist.template line 37: > 35: true > 36: NSMicrophoneUsageDescription > 37: The application is requesting access to the microphone. Should not it include the "application name" here? This is different from the java tool, where we do not know that info in advance. - PR: https://git.openjdk.java.net/jdk/pull/5186
what does the spec say about file paths that are too long?
I’ve been looking for some specification of what happens when a file request is made using a path that is too long (whatever that means to the underlying system). I have not found one. Is there one? My opinion is that silent truncation should not happen. Instead, an exception should be thrown. I have had the misfortunate of using an application (not Java) that did not check path lengths and the OS truncated the paths. The result was a kind of corruption to an implied schema causing incorrect search results and potentially data loss. Writing defensively to protect against silent path truncation is probably quite challenging, as well as tedious, especially when working with a system-independent API. If it is not already covered, I would like to see the specification mandate exceptions instead of silent truncation.
Re: RFR: JDK-8272639: jpackaged applications using microphone on mac
On Thu, 19 Aug 2021 14:12:00 GMT, Andy Herrick wrote: > JDK-8272639: jpackaged applications using microphone on mac Marked as reviewed by almatvee (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5186
Re: RFR: 8271225: Add floorDivExact() method to java.lang.[Strict]Math [v3]
On Thu, 5 Aug 2021 18:57:42 GMT, Brian Burkhalter wrote: >> Add methods `floorDivExact(int,int)` and `floorDivExact(long,long)` to >> `java.lang.Math` and `java.lang.StrictMath`. > > Brian Burkhalter has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains five commits: > > - 8271225: Verbiage update after 8271599 > - Merge > - 8271225: Revert drive-by verbiage updates > - 8271225: Some verbiage updates > - 8271225: Add floorDivExact() method to java.lang.[Strict]Math The CSR for this PR has been approved. - PR: https://git.openjdk.java.net/jdk/pull/4941
Re: RFR: 8247980: Exclusive execution of java/util/stream tests slows down tier1
On Thu, 19 Aug 2021 15:15:31 GMT, Aleksey Shipilev wrote: > See the bug report for more details. I would appreciate if people with their > corporate testing systems would run this through their systems to avoid > surprises. (ping @RealCLanger, @iignatev). Struggling to recall all the details. I think in part the tests were made exclusive to avoid the greedy parallel tests potentially competing with the execution of other greedy parallel stream tests and other independent tests. The issue may be petrubed when using machines with a large number of cores, as cost of greedly using all hardware threads is counter productive to the data sizes. It may be useful if we could limit the parallelism of stream tests on machines with large cores. I don't recall if we currently do that or have explored that. There may be no ideal configuration that fits machines with small and large cores. If there is a prioritization i would lean towards optimization by default for the automated test infra with options to modify that default for others running the tests. - PR: https://git.openjdk.java.net/jdk/pull/5189
Re: RFR: 8247980: Exclusive execution of java/util/stream tests slows down tier1
On 19/08/2021 17:10, Igor Ignatyev wrote: On Thu, 19 Aug 2021 15:15:31 GMT, Aleksey Shipilev wrote: See the bug report for more details. I would appreciate if people with their corporate testing systems would run this through their systems to avoid surprises. (ping @RealCLanger, @iignatev). Hi @shipilev , I've submitted testing for this patch in our system. meanwhile, I'd like to hear @PaulSandoz's and/or @AlanBateman's option on this as I don't think that the underlying issue, that forced us to add these tests to `exclusiveAccess.dirs` by [8058204](https://bugs.openjdk.java.net/browse/JDK-8058204), was resolved. These tests use all cores so were historically problematic with several agents VMs running concurrently, lots of timeouts. I don't recall if the reason for putting then on exclusiveAccess.dirs was release or debug builds. Getting fresh results would be useful. -Alan
Re: RFR: 8247980: Exclusive execution of java/util/stream tests slows down tier1
On Thu, 19 Aug 2021 15:15:31 GMT, Aleksey Shipilev wrote: > See the bug report for more details. I would appreciate if people with their > corporate testing systems would run this through their systems to avoid > surprises. (ping @RealCLanger, @iignatev). Hi @shipilev , I've submitted testing for this patch in our system. meanwhile, I'd like to hear @PaulSandoz's and/or @AlanBateman's option on this as I don't think that the underlying issue, that forced us to add these tests to `exclusiveAccess.dirs` by [8058204](https://bugs.openjdk.java.net/browse/JDK-8058204), was resolved. Thanks, -- Igor - PR: https://git.openjdk.java.net/jdk/pull/5189
Integrated: 8272616: Strange code in java.text.DecimalFormat#applyPattern
On Wed, 18 Aug 2021 20:59:20 GMT, Andrey Turbanov wrote: > remove redundant if This pull request has now been integrated. Changeset: 51c1b9a6 Author:Andrey Turbanov Committer: Brian Burkhalter URL: https://git.openjdk.java.net/jdk/commit/51c1b9a6870bd9644e92227e47082a53e2d1c066 Stats: 11 lines in 1 file changed: 1 ins; 3 del; 7 mod 8272616: Strange code in java.text.DecimalFormat#applyPattern Reviewed-by: bpb, naoto, iris - PR: https://git.openjdk.java.net/jdk/pull/5171
RFR: 8247980: Exclusive execution of java/util/stream tests slows down tier1
See the bug report for more details. I would appreciate if people with their corporate testing systems would run this through their systems to avoid surprises. (ping @RealCLanger, @iignatev). - Commit messages: - Drop stream tests exclusivity Changes: https://git.openjdk.java.net/jdk/pull/5189/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5189&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8247980 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5189.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5189/head:pull/5189 PR: https://git.openjdk.java.net/jdk/pull/5189
RFR: JDK-8272639: jpackaged applications using microphone on mac
JDK-8272639: jpackaged applications using microphone on mac - Commit messages: - JDK-8272639: jpackaged applications using microphone on mac Changes: https://git.openjdk.java.net/jdk/pull/5186/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5186&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8272639 Stats: 5 lines in 3 files changed: 4 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5186.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5186/head:pull/5186 PR: https://git.openjdk.java.net/jdk/pull/5186
Process for adding default method to javax.naming.Context?
Hello; I am new to this mailing list and its conventions; if I misstep I am
happy to be pointed in the right direction. Disclaimer in case it matters:
I work for Oracle on project Helidon.
I would like to understand how to request an enhancement to the
javax.naming.Context interface. A prior discussion on this subject from
many years ago can be found here:
https://mail.openjdk.java.net/pipermail/core-libs-dev/2016-October/044137.html
To recap in brief: being able to do:
Frob frob = someNamingContext.lookup("frob", Frob.class);
would be very convenient, e.g. via something like:
// Proposed new default method in javax.naming.Context
default T lookup(Name name, Class type) {
return type.cast(lookup(name)); // just delegate to the existing
lookup() method
}
Should I simply file an RFE in https://bugs.openjdk.java.net/ or should I
first further discuss the idea here?
The contribution guide (https://openjdk.java.net/contribute/) seemed to
suggest starting with a discussion on the list when an RFE doesn't already
exist, but to be honest there's not much more to the idea than this at the
moment.
Best,
Laird
Re: RFR: 8268764: Use Long.hashCode() instead of int-cast where applicable [v4]
On Thu, 1 Jul 2021 12:19:53 GMT, Сергей Цыпанов
wrote:
>> In some JDK classes there's still the following hashCode() implementation:
>>
>> long objNum;
>>
>> public int hashCode() {
>> return (int) objNum;
>> }
>>
>> This outdated expression should be replaced with Long.hashCode(long) as it
>>
>> - uses all bits of the original value, does not discard any information
>> upfront. For example, depending on how you are generating the IDs, the upper
>> bits could change more frequently (or the opposite).
>>
>> - does not introduce any bias towards values with more ones (zeros), as it
>> would be the case if the two halves were combined with an OR (AND) operation.
>>
>> See https://stackoverflow.com/a/4045083
>>
>> This is related to https://github.com/openjdk/jdk/pull/4309
>
> Сергей Цыпанов has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8268764: Update copy-right year
Let's wait, bridgekeeper.
-
PR: https://git.openjdk.java.net/jdk/pull/4491
[A linked list data structure running some operations in O(sqrt(n)) time instead of O(n)]
Hello, I was kindly directed to this mailing list in order to discuss this data structure: https://github.com/coderodde/LinkedList The README of that repository contains a table of running times for different methods and different List implementations (ArrayList, LinkedList, my version of the LinkedList). Basically, if all we want to do is to add/remove from the head/tail of a List, my implementation won't provide any performance gain. However, in case we want to work on the List in a versatile way (many add(int), addFirst/addLast, get(int), remove(int), removeFirst, removeLast, addAll(int, coll)) my version beats that of java.util package by a factor of 35 in a benchmark I wrote. I understand that ArrayList/LinkedList have a status of legacy code in JDK, but how about including my work with a different name (perhaps, FingerList or something like IndexedList/IndexedLinkedList)? Now, I would like to discuss my work with you. Did I write mature JDK code? Or, can it make it to the JDK in the first place? Best regards, rodde
Re: RFR: 8269373: some tests in jdk/tools/launcher/ fails on localized Windows platform [v2]
On Wed, 4 Aug 2021 07:25:06 GMT, Masanori Yano wrote: >> Hi all, >> >> Could you please review the 8269373 bug fixes? >> >> These tests call java.lang.ProcessBuilder in direct, so not used jtreg >> command option. To run non-localized tests, -Duser.language=en and >> -Duser.country=US options should be added in ProcessBuilder. > > Masanori Yano has updated the pull request incrementally with one additional > commit since the last revision: > > 8269373: use test opts for process arguments It is true that some other tests say Passed if the locale is not US, but I rather think that is not a good way. I think we should run tests as much as possible to ensure quality even in non-US environments. - PR: https://git.openjdk.java.net/jdk/pull/4594
Re: [(Much) faster java.util.LinkedList]
Oh, just fyi, remember to send to core-libs-dev@openjdk.java.net as well; gmail removes the mailing list when you reply by default, so our discussion like these are not shared there. We might need to check the common use cases of linked list in other places (since jdk has largely eliminated it); I think it's mainly used as a deque and a small-sized collection (as arrays for both array deque/list may have empty slots and copying on expansion is costly). We might need to see how this change impacts those usages. On Thu, Aug 19, 2021 at 12:17 AM Rodion Efremov wrote: > Hello, > > True. But could you consider the following table: > > > https://github.com/coderodde/LinkedList/blob/main/README.md#running-time-comparison > > Best regards, > rodde >
