Re: RFR: 8273111: Default timezone should return zone ID if /etc/localtime is valid but not canonicalization on linux [v5]
On Tue, 19 Oct 2021 04:08:12 GMT, Wu Yan wrote: >> Hi, >> Please help me review the change to enhance getting time zone ID from >> /etc/localtime on linux. >> >> We use `realpath` instead of `readlink` to obtain the link name of >> /etc/localtime, because `readlink` can only read the value of a symbolic of >> link, not the canonicalized absolute pathname. >> >> For example, the value of /etc/localtime is >> "../usr/share/zoneinfo//Asia/Shanghai", then the linkbuf obtained by >> `readlink` is "../usr/share/zoneinfo//Asia/Shanghai", and then the call of >> `getZoneName(linkbuf)` will get "/Asia/Shanghai", not "Asia/Shanghai", which >> consider as invalid in `ZoneInfoFile.getZoneInfo()`. Using `realpath`, you >> can get “/usr/share/zoneinfo/Asia/Shanghai“ directly from “/etc/localtime“. >> >> Thanks, >> wuyan > > Wu Yan has updated the pull request incrementally with one additional commit > since the last revision: > > fix code style looks good - Marked as reviewed by mli (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5327
Re: RFR: 7008363: TEST_BUG: test/java/lang/StringCoding/CheckEncodings.sh does nothing and is very slow at that
On Mon, 18 Oct 2021 20:49:07 GMT, Naoto Sato wrote: > Removing a problem-listed test case, which has little value in itself. > Confirmed it did succeed on all platforms before the removal. Marked as reviewed by whuang (Author). - PR: https://git.openjdk.java.net/jdk/pull/5996
Integrated: 8245095: Implementation of JEP 408: Simple Web Server
On Tue, 14 Sep 2021 08:52:37 GMT, Julia Boes wrote: > This change implements a simple web server that can be run on the > command-line with `java -m jdk.httpserver`. > > This is facilitated by adding an entry point for the `jdk.httpserver` module, > an implementation class whose main method is run when the above command is > executed. This is the first such module entry point in the JDK. > > The server is a minimal HTTP server that serves the static files of a given > directory, similar to existing alternatives on other platforms and convenient > for testing, development, and debugging. > > Additionally, a small API is introduced for programmatic creation and > customization. > > Testing: tier1-3. This pull request has now been integrated. Changeset: 9d191fce Author:Julia Boes URL: https://git.openjdk.java.net/jdk/commit/9d191fce55fa70d6a2affc724fad57b0e20e4bde Stats: 7181 lines in 42 files changed: 7146 ins; 15 del; 20 mod 8245095: Implementation of JEP 408: Simple Web Server Co-authored-by: Julia Boes Co-authored-by: Chris Hegarty Co-authored-by: Michael McMahon Co-authored-by: Daniel Fuchs Co-authored-by: Jan Lahoda Co-authored-by: Ivan Šipka Reviewed-by: ihse, jlaskey, michaelm, chegar, dfuchs - PR: https://git.openjdk.java.net/jdk/pull/5505
JDK-8275509: (jlink) SystemModulesPlugin generates a jdk.internal.module.SystemModules$all.class which isn't reproducible
This relates to the intermittent failures in
tools/jlink/JLinkReproducibleTest.java test case which has been
ProblemListed for a while now. The root cause is
https://bugs.openjdk.java.net/browse/JDK-8275509. I couldn't find any
specific mailing lists for jlink tool and I remember seeing
jlink/jpackage related discussions on this mailing list previously, so
creating this discussion here.
The jlink tool uses plugins to let them transform contents of the
modules that are part of the image. One such plugin is the
SystemModulesPlugin and as per its javadoc specification its role is to
prevent parsing of module-info.class files during JVM startup. To do so,
it creates specialized dynamically generated class files (during jlink).
The class file(s) are then bundled into the image that gets generated.
One such class file is a dynamically generated
jdk/internal/module/SystemModules$all.class. The SystemModulesPlugin
generates the bytecode for this class. The details of that bytecode are
very implementation specific. One part of the bytecode generation
involves bytecode for an internal method called "moduleDescriptors()".
One of the statements generated for this method implementation is a call
to jdk.internal.module.Builder.build(int hashCode) method[1]. What this
call does is, it creates a (pre-populated/validated) instance of the
java.lang.module.ModuleDescriptor.
The SystemModulesPlugin, when generating the bytecode of this method,
uses the hashCode() of the ModuleDescriptor of the current JVM instance
(through a typical ModuleDescriptor#hashCode() call)[2]. By doing so, it
ends up generating bytecode which "embeds" the current runtime specific
hashcode into the generated class file.
The contract of java.lang.Object#hashCode() states:
"
...
This integer need not remain consistent from one execution of an
application to another execution of the same application.
"
Effectively, what this means is, if jlink is used to generate an image
with the exact same set of modules (requirements, package, exports,
opens etc...) it still can (and does) end up generating a
jdk/internal/module/SystemModules$all.class file whose binary content
will differ across these runs, thus being non-reproducible.
The implementation in java.lang.module.ModuleDescriptor is such that if
the hashcode passed to it (during construction) is 0, then it lazily
computes the correct hashcode whenever the invocation of hashCode()
happens at runtime. What that means is, the SystemModulesPlugin in its
bytecode generation for the moduleDescriptors() method could always pass
0 as the hashcode to the jdk.internal.module.Builder.build(int hashCode)
call. That's something that I experimented with and after that change,
with 100s of runs of the JLinkReproducibleTest, I no longer get any
failures. However, given that the SystemModulesPlugin's goal appears to
be to reduce the booting time for system modules, I was wondering if
this change would introduce any performance penalty that is big enough.
What this change will end up doing is, whenever the next time the
hashCode method on instances of the system module's ModuleDescriptor
gets called, it will have to compute it and that computation is
relatively expensive (given how many "components" it uses to calculate
it[3]). It's a (mostly) one time thing for each instance, but I don't
know how expensive that would be. Do we have any existing benchmarks in
this area that I could reuse to see what performance impact this might
have? Keeping aside the performance issue for a bit, is this proposed
patch something worth considering:
---
a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java
+++
b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java
@@ -1099,7 +1099,13 @@ public final class SystemModulesPlugin extends
AbstractPlugin {
mv.visitVarInsn(ALOAD, MD_VAR);
pushInt(mv, index);
mv.visitVarInsn(ALOAD, BUILDER_VAR);
- mv.visitLdcInsn(md.hashCode());
+ // Let the ModuleDescriptor hashcode be computed at
runtime.
+ // Embedding the current hashcode of the ModuleDescriptor
+ // into the bytecode of a generated class can cause the
generated
+ // bytecode to be not reproducible, since an object's
hashcode is allowed
+ // to change across JVM runs.
+ mv.visitLdcInsn(0); // the hashcode to be passed to the
+ //
jdk.internal.module.Builder.build(int) method
mv.visitMethodInsn(INVOKEVIRTUAL,
MODULE_DESCRIPTOR_BUILDER,
"build", "(I)Ljava/lang/module/ModuleDescriptor;",
false);
The other option I experimented with was to make
ModuleDescriptor#hashCode() generate the same hashcode across multiple
JVM runs. Although I do have a "worki
RFR: 8268595: java/io/Serializable/serialFilter/GlobalFilterTest.java#id1 failed in timeout
The `GlobalFilterTest` has to 2 `@test` tags. One of them has failed with a timeout as noted in https://bugs.openjdk.java.net/browse/JDK-8268595. The timeout seems to have happened even after the tests had already completed successfully. Like I note in the JBS comments of that issue, I suspect it might have to do with the "-XX:StartFlightRecording:name=DeserializationEvent,dumponexit=true" usage in this test action. The commit in this PR removes that second `@test` altogether because (correct me if I'm wrong) from what I understand, this test never enables the DeserializationEvent which means there is no JFR events being captured for deserialization in this test, nor does the test do any JFR events related testing. So, I think this second `@test` is virtually a no-op when it comes to the JFR testing. There's a separate `TestDeserializationEvent` which has a comprehensive testing of the DeserializationEvent. - Commit messages: - 8268595: java/io/Serializable/serialFilter/GlobalFilterTest.java#id1 failed in timeout Changes: https://git.openjdk.java.net/jdk/pull/6008/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6008&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8268595 Stats: 12 lines in 1 file changed: 0 ins; 12 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6008.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6008/head:pull/6008 PR: https://git.openjdk.java.net/jdk/pull/6008
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v4]
> This change implements a new service provider interface for host name and > address resolution, so that java.net.InetAddress API can make use of > resolvers other than the platform's built-in resolver. > > The following API classes are added to `java.net.spi` package to facilitate > this: > - `InetAddressResolverProvider` - abstract class defining a service, and is, > essentially, a factory for `InetAddressResolver` resolvers. > - `InetAddressResolverProvider.Configuration ` - an interface describing the > platform's built-in configuration for resolution operations that could be > used to bootstrap a resolver construction, or to implement partial delegation > of lookup operations. > - `InetAddressResolver` - an interface that defines methods for the > fundamental forward and reverse lookup operations. > - `InetAddressResolver.LookupPolicy` - a class whose instances describe the > characteristics of one forward lookup operation. > > More details in [JEP-418](https://openjdk.java.net/jeps/418). > > Testing: new and existing `tier1:tier3` tests Aleksei Efimov has updated the pull request incrementally with four additional commits since the last revision: - Remove no longer used import from IPSupport - Rename IPSupport.hasAddress and update it to use SocketChannel.open - Address review comments: javadoc + code cleanup - Address resolver bootstraping issue - Changes: - all: https://git.openjdk.java.net/jdk/pull/5822/files - new: https://git.openjdk.java.net/jdk/pull/5822/files/d302a49a..ac0d2184 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=02-03 Stats: 245 lines in 5 files changed: 180 ins; 35 del; 30 mod Patch: https://git.openjdk.java.net/jdk/pull/5822.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5822/head:pull/5822 PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v3]
On Fri, 15 Oct 2021 14:25:02 GMT, Daniel Fuchs wrote:
>> Aleksei Efimov has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Add @since tags to new API classes
>> - Add checks and test for empty stream resolver results
>
> src/java.base/share/classes/java/net/InetAddress.java line 231:
>
>> 229: * The first provider found will be used to instantiate the {@link
>> InetAddressResolver InetAddressResolver}
>> 230: * by invoking the {@link
>> InetAddressResolverProvider#get(InetAddressResolverProvider.Configuration)}
>> 231: * method. The instantiated {@code InetAddressResolver} will be
>> installed as the system-wide
>
> Maybe "... method. The **returned** {@code InetAddressResolver} will be
> installed ..."
Changed as suggested in 648e65b8e6ae9687f7164ea3e81c51ab2b5e2dfe.
> src/java.base/share/classes/java/net/InetAddress.java line 486:
>
>> 484: return cns;
>> 485: } finally {
>> 486: bootstrapResolver = null;
>
> This is incorrect. This will set bootstrapResolver to null in the case where
> you return it at line 468 - which means that a second call will then find it
> null. I believe you want to set it to null in the finally clause only if you
> reached line 470. You could achieve this by declaring a local variable - e.g
> `InetAddressResolver tempResolver = null;` before entering the try-finally
> then set that variable at line 470 `return tempResolver = bootstrapResolver;`
> and in finally do `if (tempResolver == null) bootsrapResolver = null;`
>
> To test this you could try to call e.g. `InetAddress.getByName` twice in
> succession in your test: I believe it will fail with the code as it stands,
> but will succeed with my proposed changes...
Good catch, thank you Daniel. Added a test for that and fixed it in
cd19ecd63838d3c2b713ed66d41107fe01a2d3e6. Now the `bootstrapResolver` field is
set back to `null` in finally block only if a call to `InetAddress.resolver()`
entered a resolver initialization part.
> src/java.base/share/classes/java/net/InetAddress.java line 860:
>
>> 858: return host;
>> 859: }
>> 860: } catch (RuntimeException | UnknownHostException e) {
>
> This may deserve a comment: resolver.lookUpHostName and
> InetAddress.getAllbyName0 delegate to the system-wide resolver, which could
> be custom, and we treat any unexpected RuntimeException thrown by the
> provider at that point as we would treat an UnknownHostException or an
> unmatched host name.
Agree - comment added as part of 648e65b8e6ae9687f7164ea3e81c51ab2b5e2dfe.
> src/java.base/share/classes/java/net/InetAddress.java line 1063:
>
>> 1061: public Stream lookupAddresses(String host,
>> LookupPolicy policy)
>> 1062: throws UnknownHostException {
>> 1063: Objects.requireNonNull(host);
>
> Should we also double check that `policy` is not null? Or maybe assert it?
Yes, we should since custom resolvers might call the built-in one with `null` -
added non-null check in 648e65b .
> src/java.base/share/classes/java/net/InetAddress.java line 1203:
>
>> 1201: + " as hosts file " + hostsFile + " not found
>> ");
>> 1202: }
>> 1203: // Check number of found addresses:
>
> I wonder if the logic here could be simplified by first checking whether only
> IPv4 addresses are requested, and then if only IPv6 addresses are requested.
>
> That is - evacuate the simple cases first and then only deal with the more
> complex case where you have to combine the two lists...
Tried to simplify it in 648e65b8e6ae9687f7164ea3e81c51ab2b5e2dfe
-
PR: https://git.openjdk.java.net/jdk/pull/5822
Re: JDK-8275509: (jlink) SystemModulesPlugin generates a jdk.internal.module.SystemModules$all.class which isn't reproducible
On 19/10/2021 13:31, Jaikiran Pai wrote: This relates to the intermittent failures in tools/jlink/JLinkReproducibleTest.java test case which has been ProblemListed for a while now. The root cause is https://bugs.openjdk.java.net/browse/JDK-8275509. I couldn't find any specific mailing lists for jlink tool and I remember seeing jlink/jpackage related discussions on this mailing list previously, so creating this discussion here. Reproducible builds have been a game of whack-a-mole. Many issues have been fixed in recent releases to the JDK is a lot better than it used to be. As it happens, someone else interested in reproducible builds brought up the issue of the hashCode on jigsaw-dev a few weeks ago [1]. The system modules plugin pre-dates the archiving of the module graph in the CDS archive so the performance profile is probably very different from when the plugin was created. I'll defer to Claes on what benchmarks to run. As regards changing the plugin then if we aren't creating the MD with a hashCode then I'd prefer to drop the hashCode parameter from the Builder and the shared secret. In other words, do some cleanup rather than leave it orfaned. -Alan [1] https://mail.openjdk.java.net/pipermail/jigsaw-dev/2021-September/014708.html
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v3]
On Fri, 15 Oct 2021 17:09:46 GMT, Daniel Fuchs wrote:
>> Aleksei Efimov has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Add @since tags to new API classes
>> - Add checks and test for empty stream resolver results
>
> test/jdk/java/net/spi/InetAddressResolverProvider/providers/recursive/recursive.init.provider/impl/InetAddressUsageInGetProviderImpl.java
> line 38:
>
>> 36: String localHostName;
>> 37: try {
>> 38: localHostName = InetAddress.getLocalHost().getHostName();
>
> Try to call InetAddress.getLocalHost().getHostName(); twice here.
New `BootstrapResolverUsageTest` test is added to trigger the bootstrap
resolver problem. `InetAddress.getByName` calls with unique names are used
instead of `InetAddress.getLocalHost()` to avoid caching of LHN resolution
results. Added in cd19ecd63838d3c2b713ed66d41107fe01a2d3e6.
-
PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v3]
On Fri, 15 Oct 2021 17:19:26 GMT, Daniel Fuchs wrote:
>> Aleksei Efimov has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Add @since tags to new API classes
>> - Add checks and test for empty stream resolver results
>
> test/lib/jdk/test/lib/net/IPSupport.java line 88:
>
>> 86: } catch (SocketException se2) {
>> 87: return false;
>> 88: }
>
> The implementation of this method now conflicts with its name. Maybe we
> should pass a `Class` as parameter, and construct the
> loopback address from there. IIRC the issue with the original implementation
> was that it would say that IPv6 was not supported if IPv6 had only been
> disabled on the loopback interface - and that caused trouble because the
> native implementation of the built-in resolver had a different view of the
> situation - and saw that an IPv6 stack was available on the machine. Maybe
> this would deserve a comment too - so that future maintainers can figure out
> what we are trying to do here.
Thanks for a good suggestion: renamed to `isSupported`, changed parameter type
to `Class addressType` and updated it to use
`SocketChannel.open(ProtocolFamily pf)`
[API](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/nio/channels/SocketChannel.html#open(java.net.ProtocolFamily))
added in `15` to check if the plafrom supports addresses of specified address
type. Pushed as 95a21e57fbe8be147d23e6a6901ae307e8237cbb.
All new tests (especially `LookupPolicyMappingTest`) pass in environment with
`IPv6` addresses disabled.
-
PR: https://git.openjdk.java.net/jdk/pull/5822
Re: JDK-8275509: (jlink) SystemModulesPlugin generates a jdk.internal.module.SystemModules$all.class which isn't reproducible
On 2021-10-19 14:31, Jaikiran Pai wrote: The other option I experimented with was to make ModuleDescriptor#hashCode() generate the same hashcode across multiple JVM runs. Although I do have a "working" version of that change, I decided not to spend too much time on it because the java.lang.Object#hashCode() contract itself clearly states that this value isn't expected to be same across multiple JVM runs. So whatever I do here is going to be brittle. I'm assuming the cause for ModuleDescriptor#hashCode being is due to the various enums not having an explicitly defined hashCode? I think this should be fixed. Either way you're going to be brittle since the patch to emit a 0 is relying on the ModuleDescriptor#hashCode implementation disallowing 0 as a hash value (a 0 will force a recalculation). While it'll only happen at most once per module these relatively expensive calculations are something we want to avoid on startup if we can do so for free. /Claes
Re: JDK-8275509: (jlink) SystemModulesPlugin generates a jdk.internal.module.SystemModules$all.class which isn't reproducible
Hello Alan,
On 19/10/21 6:59 pm, Alan Bateman wrote:
On 19/10/2021 13:31, Jaikiran Pai wrote:
This relates to the intermittent failures in
tools/jlink/JLinkReproducibleTest.java test case which has been
ProblemListed for a while now. The root cause is
https://bugs.openjdk.java.net/browse/JDK-8275509. I couldn't find any
specific mailing lists for jlink tool and I remember seeing
jlink/jpackage related discussions on this mailing list previously,
so creating this discussion here.
Reproducible builds have been a game of whack-a-mole. Many issues have
been fixed in recent releases to the JDK is a lot better than it used
to be. As it happens, someone else interested in reproducible builds
brought up the issue of the hashCode on jigsaw-dev a few weeks ago [1].
Ah! So this exact same investigation had already happened a few weeks
back then. I haven't subscribed to that list, so missed it. I see in one
of those messages this part:
"Off hand I can't think of any issues with the ModuleDescriptor
hashCode. It is computed at link time and should be deterministic. If I
were to guess then then this may be something to do with the module
version recorded at compile-time at that is one of the components that
the hash is based on."
To be clear, is the ModuleDescriptor#hashCode() expected to return
reproducible (same) hashCode across multiple runs? What currently
changes the hashCode() across multiple runs is various components within
ModuleDescriptor's hashCode() implementation using the hashCode() of the
enums (specifically the various Modifier enums). For example here
https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/lang/module/ModuleDescriptor.java#L330
(the mods.hashCode()). Since the hashCode() returned by enums is
literally through a call to java.lang.Object#hashCode(), those
hashCode() value ended up changing across JVM runs, in one of the setups
I was testing (which I didn't consider a surprise since that's what the
Object#hashCode() stated).
The other approach that I talked about in my previous mail of trying to
make ModuleDescriptor#hashCode() reproducible involved using the enum's
ordinal value as a part of the hashCode() computation instead of calling
the enum's hashCode() method. Very crudely, it looked like:
diff --git
a/src/java.base/share/classes/java/lang/module/ModuleDescriptor.java
b/src/java.base/share/classes/java/lang/module/ModuleDescriptor.java
index a412dd753cc..13c8cd04360 100644
--- a/src/java.base/share/classes/java/lang/module/ModuleDescriptor.java
+++ b/src/java.base/share/classes/java/lang/module/ModuleDescriptor.java
@@ -327,7 +327,7 @@ public class ModuleDescriptor
*/
@Override
public int hashCode() {
- int hash = name.hashCode() * 43 + mods.hashCode();
+ int hash = name.hashCode() * 43 + enumOrdinalHashCode(mods);
if (compiledVersion != null)
hash = hash * 43 + compiledVersion.hashCode();
if (rawCompiledVersion != null)
@@ -505,7 +505,7 @@ public class ModuleDescriptor
*/
@Override
public int hashCode() {
- int hash = mods.hashCode();
+ int hash = enumOrdinalHashCode(mods);
hash = hash * 43 + source.hashCode();
return hash * 43 + targets.hashCode();
}
@@ -708,7 +708,7 @@ public class ModuleDescriptor
*/
@Override
public int hashCode() {
- int hash = mods.hashCode();
+ int hash = enumOrdinalHashCode(mods);
hash = hash * 43 + source.hashCode();
return hash * 43 + targets.hashCode();
}
@@ -2261,7 +2261,7 @@ public class ModuleDescriptor
int hc = hash;
if (hc == 0) {
hc = name.hashCode();
- hc = hc * 43 + Objects.hashCode(modifiers);
+ hc = hc * 43 + enumOrdinalHashCode(modifiers);
hc = hc * 43 + requires.hashCode();
hc = hc * 43 + Objects.hashCode(packages);
hc = hc * 43 + exports.hashCode();
@@ -2546,6 +2546,21 @@ public class ModuleDescriptor
.collect(Collectors.joining(" "));
}
+ /**
+ * Generates and returns a hashcode for the enum instances. The
returned hashcode
+ * is a sum of each of the enum instances' {@link Enum#ordinal()
ordinal} value.
+ */
+ private static int enumOrdinalHashCode(final IterableEnum> enums) {
+ int h = 0;
+ for (final Enum e : enums) {
+ if (e == null) {
+ continue;
+ }
+ h += e.ordinal();
+ }
+ return h;
+ }
+
private static >
int compare(T obj1, T obj2) {
if (obj1 != null) {
With this change (and this change only, no changes in
SystemModulesPlugin were needed) I was able to consistently run that
test without any failures. But I didn't pursue this effort further
because I thought making ModuleDesc
Re: RFR: 8269336: Malformed jdk.serialFilter incorrectly handled
On Mon, 18 Oct 2021 13:44:56 GMT, Jaikiran Pai wrote:
> Can I please get a review for this change which addresses
> https://bugs.openjdk.java.net/browse/JDK-8269336?
>
> As noted in that issue, this change will now propagate any exception that
> occurred during parsing and creation of the filter configured through the
> `jdk.serialFilter` system property. It will also continue to log those
> errors, like it previously did.
>
> A new jtreg test has been introduced to reproduce this issue and verify the
> fix.
>
> Given that invalid values for this system property will now start throwing
> exception, will this change need a CSR?
src/java.base/share/classes/java/io/ObjectInputFilter.java line 641:
> 639: "Error configuring filter: {0}", (Object)
> re);
> 640: // Do not continue if configuration not initialized
> 641: throw new ExceptionInInitializerError(re);
Just re-throw the RuntimeException.
It will be caught by the caller of the static initialize and wrapped in
ExceptioninInitializerError.
I don't think a CSR is needed since this exception is not part of an API.
-
PR: https://git.openjdk.java.net/jdk/pull/5988
Re: JDK-8275509: (jlink) SystemModulesPlugin generates a jdk.internal.module.SystemModules$all.class which isn't reproducible
Hello Claes, On 19/10/21 7:07 pm, Claes Redestad wrote: On 2021-10-19 14:31, Jaikiran Pai wrote: The other option I experimented with was to make ModuleDescriptor#hashCode() generate the same hashcode across multiple JVM runs. Although I do have a "working" version of that change, I decided not to spend too much time on it because the java.lang.Object#hashCode() contract itself clearly states that this value isn't expected to be same across multiple JVM runs. So whatever I do here is going to be brittle. I'm assuming the cause for ModuleDescriptor#hashCode being is due to the various enums not having an explicitly defined hashCode? You are right. That was what was causing the change in values. I just sent a separate reply in this thread with additional details. I think this should be fixed. Okay, I'll pursue this path then. Either way you're going to be brittle since the patch to emit a 0 is relying on the ModuleDescriptor#hashCode implementation disallowing 0 as a hash value (a 0 will force a recalculation). While it'll only happen at most once per module these relatively expensive calculations are something we want to avoid on startup if we can do so for free. Understood. Thank you for these inputs. -Jaikiran
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v3]
On Sun, 17 Oct 2021 21:39:06 GMT, Mark Sheppard wrote: > I think that a hostname is constant while a host is up, but it can be > changed, and when changed a host restart is required. I don't think it is > quite as dynamic as has been suggested, but I open to correction. It is possible to change a host name without host restart. What has been tested: - Checked O/S type - Linux - `hostnamectl` cmd was used to change a host name - In output below there is only one `jshell` instance running. jshell> InetAddress.getLocalHost() $1 ==> test-host-name/192.168.0.155 $ hostnamectl set-hostname 'test-host-name-changed' # Need to sleep for 5 seconds since local host # is cached for 5 seconds since last resolution $ sleep 5 jshell> InetAddress.getLocalHost() $2 ==> test-host-name-changed/192.168.0.155 I believe it proves that we need to treat a host name as dynamically changing information. - PR: https://git.openjdk.java.net/jdk/pull/5822
RFR: 8275512: Upgrade required version of jtreg to 6.1
As a follow up of JEP 411, we will soon disallow security manager by default. jtreg 6.1 does not set its own security manager if JDK version is >= 18. - Commit messages: - 8275512: Upgrade required version of jtreg to 6.1 Changes: https://git.openjdk.java.net/jdk/pull/6012/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6012&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275512 Stats: 7 lines in 6 files changed: 0 ins; 0 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/6012.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6012/head:pull/6012 PR: https://git.openjdk.java.net/jdk/pull/6012
Re: RFR: 8275512: Upgrade required version of jtreg to 6.1
On Tue, 19 Oct 2021 13:51:45 GMT, Weijun Wang wrote: > As a follow up of JEP 411, we will soon disallow security manager by default. > jtreg 6.1 does not set its own security manager if JDK version is >= 18. LGTM - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6012
Re: JDK-8275509: (jlink) SystemModulesPlugin generates a jdk.internal.module.SystemModules$all.class which isn't reproducible
On 19/10/2021 14:49, Jaikiran Pai wrote: Ah! So this exact same investigation had already happened a few weeks back then. I haven't subscribed to that list, so missed it. I see in one of those messages this part: "Off hand I can't think of any issues with the ModuleDescriptor hashCode. It is computed at link time and should be deterministic. If I were to guess then then this may be something to do with the module version recorded at compile-time at that is one of the components that the hash is based on." To be clear, is the ModuleDescriptor#hashCode() expected to return reproducible (same) hashCode across multiple runs? What currently changes the hashCode() across multiple runs is various components within ModuleDescriptor's hashCode() implementation using the hashCode() of the enums (specifically the various Modifier enums). The discussion on jigsaw-dev didn't get to the bottom of the issue at the time, mostly because it wasn't easy to reproduce. Now that the issue is clearer then we should fix it. Aside from reproducible builds then I expect it is possible to use a ModuleDescriptor.Builder to create a ModuleDescriptor that is equal to a ModuleDescriptor in boot layer configuration but with a different hashCode. On the surface, changing the MD hash code to use the ordinal of the modifiers should be okay. If the enums values are re-ordered then it should also be okay but clearly doing so would end up with a build that is not binary identical to a build done with a different order. I think we need to think though if there are any implications. -Alan
Re: RFR: 8269336: Malformed jdk.serialFilter incorrectly handled [v2]
> Can I please get a review for this change which addresses > https://bugs.openjdk.java.net/browse/JDK-8269336? > > As noted in that issue, this change will now propagate any exception that > occurred during parsing and creation of the filter configured through the > `jdk.serialFilter` system property. It will also continue to log those > errors, like it previously did. > > A new jtreg test has been introduced to reproduce this issue and verify the > fix. > > Given that invalid values for this system property will now start throwing > exception, will this change need a CSR? Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision: Roger's review suggestion - rethrow the RuntimeException instead of wrapping in ExceptionInInitializerError - Changes: - all: https://git.openjdk.java.net/jdk/pull/5988/files - new: https://git.openjdk.java.net/jdk/pull/5988/files/3169a338..d443049a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5988&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5988&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5988.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5988/head:pull/5988 PR: https://git.openjdk.java.net/jdk/pull/5988
Re: RFR: 8269336: Malformed jdk.serialFilter incorrectly handled [v2]
On Tue, 19 Oct 2021 13:48:33 GMT, Roger Riggs wrote:
>> Jaikiran Pai has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Roger's review suggestion - rethrow the RuntimeException instead of
>> wrapping in ExceptionInInitializerError
>
> src/java.base/share/classes/java/io/ObjectInputFilter.java line 641:
>
>> 639: "Error configuring filter: {0}", (Object)
>> re);
>> 640: // Do not continue if configuration not initialized
>> 641: throw new ExceptionInInitializerError(re);
>
> Just re-throw the RuntimeException.
> It will be caught by the caller of the static initialize and wrapped in
> ExceptioninInitializerError.
>
> I don't think a CSR is needed since this exception is not part of an API.
Done. Updated the PR. Test continues to pass.
I had copy/pasted the wrapping into `ExceptioninInitializerError` from a few
lines down, in that code. But looking at it more closely now, I guess, that
other line which is throwing the `ExceptioninInitializerError` is doing that
because what's being thrown in that block can potentially be a `Throwable`.
-
PR: https://git.openjdk.java.net/jdk/pull/5988
Re: RFR: 8275512: Upgrade required version of jtreg to 6.1
On Tue, 19 Oct 2021 13:51:45 GMT, Weijun Wang wrote: > As a follow up of JEP 411, we will soon disallow security manager by default. > jtreg 6.1 does not set its own security manager if JDK version is >= 18. LGTM - Marked as reviewed by iignatyev (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6012
Integrated: 7008363: TEST_BUG: test/java/lang/StringCoding/CheckEncodings.sh does nothing and is very slow at that
On Mon, 18 Oct 2021 20:49:07 GMT, Naoto Sato wrote: > Removing a problem-listed test case, which has little value in itself. > Confirmed it did succeed on all platforms before the removal. This pull request has now been integrated. Changeset: 8a3e0a1f Author:Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/8a3e0a1fc1fef02edf9621b13e8be8b96a12bb0f Stats: 96 lines in 3 files changed: 0 ins; 96 del; 0 mod 7008363: TEST_BUG: test/java/lang/StringCoding/CheckEncodings.sh does nothing and is very slow at that Reviewed-by: iris, lancea, bpb, whuang - PR: https://git.openjdk.java.net/jdk/pull/5996
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v4]
On Tue, 19 Oct 2021 13:28:26 GMT, Aleksei Efimov wrote: >> This change implements a new service provider interface for host name and >> address resolution, so that java.net.InetAddress API can make use of >> resolvers other than the platform's built-in resolver. >> >> The following API classes are added to `java.net.spi` package to facilitate >> this: >> - `InetAddressResolverProvider` - abstract class defining a service, and >> is, essentially, a factory for `InetAddressResolver` resolvers. >> - `InetAddressResolverProvider.Configuration ` - an interface describing the >> platform's built-in configuration for resolution operations that could be >> used to bootstrap a resolver construction, or to implement partial >> delegation of lookup operations. >> - `InetAddressResolver` - an interface that defines methods for the >> fundamental forward and reverse lookup operations. >> - `InetAddressResolver.LookupPolicy` - a class whose instances describe the >> characteristics of one forward lookup operation. >> >> More details in [JEP-418](https://openjdk.java.net/jeps/418). >> >> Testing: new and existing `tier1:tier3` tests > > Aleksei Efimov has updated the pull request incrementally with four > additional commits since the last revision: > > - Remove no longer used import from IPSupport > - Rename IPSupport.hasAddress and update it to use SocketChannel.open > - Address review comments: javadoc + code cleanup > - Address resolver bootstraping issue Changes look good to me. - Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5822
RFR: 8272614: Unused parameters in MethodHandleNatives linking methods
Please review this small fix for JDK-8272614 to remove the unused indexInCP argument to linkCallSite() and linkDynamicConstant(). The fix was tested with Mach5 tiers 1-2 on Linux, Mac OS, and Windows, and Mach5 tiers 3-6 on Linux x64. Thanks, Harold - Commit messages: - 8272614: Unused parameters in MethodHandleNatives linking methods Changes: https://git.openjdk.java.net/jdk/pull/6021/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6021&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8272614 Stats: 8 lines in 3 files changed: 0 ins; 3 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/6021.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6021/head:pull/6021 PR: https://git.openjdk.java.net/jdk/pull/6021
Re: RFR: 8275512: Upgrade required version of jtreg to 6.1 [v2]
> As a follow up of JEP 411, we will soon disallow security manager by default. > jtreg 6.1 does not set its own security manager if JDK version is >= 18. Weijun Wang has updated the pull request incrementally with one additional commit since the last revision: upgrade the version in GHA config only in patch2: unchanged: - Changes: - all: https://git.openjdk.java.net/jdk/pull/6012/files - new: https://git.openjdk.java.net/jdk/pull/6012/files/b86e799c..f5ffc49b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6012&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6012&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/6012.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6012/head:pull/6012 PR: https://git.openjdk.java.net/jdk/pull/6012
Re: RFR: 8275512: Upgrade required version of jtreg to 6.1 [v2]
On Tue, 19 Oct 2021 17:24:17 GMT, Weijun Wang wrote: >> As a follow up of JEP 411, we will soon disallow security manager by >> default. jtreg 6.1 does not set its own security manager if JDK version is >> >= 18. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > upgrade the version in GHA config > > only in patch2: > unchanged: Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6012
Re: RFR: 8275512: Upgrade required version of jtreg to 6.1 [v2]
On Tue, 19 Oct 2021 17:24:17 GMT, Weijun Wang wrote: >> As a follow up of JEP 411, we will soon disallow security manager by >> default. jtreg 6.1 does not set its own security manager if JDK version is >> >= 18. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > upgrade the version in GHA config > > only in patch2: > unchanged: Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6012
Re: RFR: 8271515: Integration of JEP 417: Vector API (Third Incubator) [v3]
On Mon, 18 Oct 2021 22:56:37 GMT, Sandhya Viswanathan
wrote:
>> Paul Sandoz has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains seven commits:
>>
>> - Merge branch 'master' into JDK-8271515-vector-api
>> - Apply patch from https://github.com/openjdk/panama-vector/pull/152
>> - Apply patch from https://github.com/openjdk/panama-vector/pull/142
>> - Apply patch from https://github.com/openjdk/panama-vector/pull/139
>> - Apply patch from https://github.com/openjdk/panama-vector/pull/151
>> - Add new files.
>> - 8271515: Integration of JEP 417: Vector API (Third Incubator)
>
> src/hotspot/share/utilities/globalDefinitions_vecApi.hpp line 29:
>
>> 27: // the intent of this file to provide a header that can be included in
>> .s files.
>> 28:
>> 29: #ifndef SHARE_VM_UTILITIES_GLOBALDEFINITIONS_VECAPI_HPP
>
> The file src/hotspot/share/utilities/globalDefinitions_vecApi.hpp is not
> needed.
I notice
src/jdk.incubator.vector/windows/native/libsvml/globals_vectorApiSupport_windows.S.inc
contains a refence in comments to that file, I presume i can remove that
comment too?
> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Byte128Vector.java
> line 278:
>
>> 276: @Override
>> 277: @ForceInline
>> 278: public Byte128Vector lanewise(Unary op, VectorMask m) {
>
> Should this method be final as well?
It's actually redundant because the class is final. Better to drop final from
all declarations, at the risk of creating a larger diff.
> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Byte128Vector.java
> line 321:
>
>> 319: public final
>> 320: Byte128Vector
>> 321: lanewise(Ternary op, Vector v1, Vector v2,
>> VectorMask m) {
>
> Should we use VectorOperators.Ternary here?
We static import `import static jdk.incubator.vector.VectorOperators.*;`, and
the qualification of enclosing type `VectorOperators` seems inconsistently
used. I think we can drop it at all declarations.
> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/ByteVector.java
> line 603:
>
>> 601: if (opKind(op, VO_SPECIAL)) {
>> 602: if (op == ZOMO) {
>> 603: return blend(broadcast(-1), compare(NE, 0, m));
>
> This doesn't look correct. The lanes where mask is false should get the
> original lane value in this vector.
That should work, since `compare(NE, 0, m) === compare(NE, 0).and(m)`, so when
an `m` lane is unset the lane element of `this` vector will be selected.
Running jshell against a build of PR:
$ ~/Projects/jdk/jdk/build/macosx-x86_64-server-release/images/jdk/bin/jshell
--add-modules jdk.incubator.vector
| Welcome to JShell -- Version 18-internal
| For an introduction type: /help intro
jshell> import jdk.incubator.vector.*
jshell> var s = IntVector.SPECIES_256;
s ==> Species[int, 8, S_256_BIT]
jshell> var v = IntVector.fromArray(s, new int[]{0, 1, 0, -2, 0, 3, 0, -4}, 0);
v ==> [0, 1, 0, -2, 0, 3, 0, -4]
jshell> var z = v.lanewise(VectorOperators.ZOMO);
z ==> [0, -1, 0, -1, 0, -1, 0, -1]
jshell> z = v.lanewise(VectorOperators.ZOMO, s.loadMask(new boolean[]{false,
false, false, false, true, true, true, true}, 0));
z ==> [0, 1, 0, -2, 0, -1, 0, -1]
jshell>
-
PR: https://git.openjdk.java.net/jdk/pull/5873
Re: RFR: 8271515: Integration of JEP 417: Vector API (Third Incubator) [v3]
On Tue, 19 Oct 2021 18:54:01 GMT, Paul Sandoz wrote:
>> src/hotspot/share/utilities/globalDefinitions_vecApi.hpp line 29:
>>
>>> 27: // the intent of this file to provide a header that can be included in
>>> .s files.
>>> 28:
>>> 29: #ifndef SHARE_VM_UTILITIES_GLOBALDEFINITIONS_VECAPI_HPP
>>
>> The file src/hotspot/share/utilities/globalDefinitions_vecApi.hpp is not
>> needed.
>
> I notice
> src/jdk.incubator.vector/windows/native/libsvml/globals_vectorApiSupport_windows.S.inc
> contains a refence in comments to that file, I presume i can remove that
> comment too?
Yes, that comment can also be removed. It is a leftover from when svml was
built as part of libjvm.so.
>> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Byte128Vector.java
>> line 278:
>>
>>> 276: @Override
>>> 277: @ForceInline
>>> 278: public Byte128Vector lanewise(Unary op, VectorMask m) {
>>
>> Should this method be final as well?
>
> It's actually redundant because the class is final. Better to drop final from
> all declarations, at the risk of creating a larger diff.
Got it. I am ok with leaving things as is if it makes it easier.
-
PR: https://git.openjdk.java.net/jdk/pull/5873
Re: RFR: 8271515: Integration of JEP 417: Vector API (Third Incubator) [v3]
On Tue, 19 Oct 2021 18:57:59 GMT, Sandhya Viswanathan wrote: >> It's actually redundant because the class is final. Better to drop final >> from all declarations, at the risk of creating a larger diff. > > Got it. I am ok with leaving things as is if it makes it easier. For now to reduce changes in this PR i would prefer to do a follow cleanup for code consistency for that and for `final` methods. - PR: https://git.openjdk.java.net/jdk/pull/5873
Withdrawn: 8275167: x86 intrinsic for unsignedMultiplyHigh
On Wed, 13 Oct 2021 18:55:10 GMT, Vamsi Parasa wrote: > Optimize the new Math.unsignedMultiplyHigh using the x86 mul instruction. > This change show 1.87X improvement on a micro benchmark. This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk/pull/5933
Re: RFR: 8275167: x86 intrinsic for unsignedMultiplyHigh [v2]
On Fri, 15 Oct 2021 21:04:12 GMT, Vamsi Parasa wrote: > > > How you verified correctness of results? I suggest to extend > > > `test/jdk//java/lang/Math/MultiplicationTests.java` test to cover > > > unsigned method. > > > > > > Tests for unsignedMultiplyHigh were already added in > > test/jdk//java/lang/Math/MultiplicationTests.java (in July 2021 by Brian > > Burkhalter). Used that test to verify the correctness of the results. > > Good. It seems I have old version of the test. Did you run it with -Xcomp? > How you verified that intrinsic is used? I have verified that the intrinsic is being used by looking at the x86 assembly code generated by using perfasm profiler. - PR: https://git.openjdk.java.net/jdk/pull/5933
Re: RFR: 8275167: x86 intrinsic for unsignedMultiplyHigh [v2]
> Optimize the new Math.unsignedMultiplyHigh using the x86 mul instruction. > This change show 1.87X improvement on a micro benchmark. Vamsi Parasa has updated the pull request incrementally with one additional commit since the last revision: refactoring to remove code duplication by using a common routine for UMulHiLNode and MulHiLNode - Changes: - all: https://git.openjdk.java.net/jdk/pull/5933/files - new: https://git.openjdk.java.net/jdk/pull/5933/files/cb30b268..a10a9fbe Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5933&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5933&range=00-01 Stats: 25 lines in 2 files changed: 12 ins; 12 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5933.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5933/head:pull/5933 PR: https://git.openjdk.java.net/jdk/pull/5933
Re: RFR: 8275167: x86 intrinsic for unsignedMultiplyHigh [v2]
On Fri, 15 Oct 2021 19:31:24 GMT, Vamsi Parasa wrote: >> src/hotspot/share/opto/mulnode.cpp line 468: >> >>> 466: } >>> 467: >>> 468: >>> //= >> >> MulHiLNode::Value() and UMulHiLNode::Value() seem to be identical. Perhaps >> some refactoring would be in order, maybe make a common shared routine. > > Sure, will do the refactoring to use a shared routine. Pushed the refactored code to use a common routine for MulHiLNode::Value() and UMulHiLNode::Value(). Please review... - PR: https://git.openjdk.java.net/jdk/pull/5933
Re: RFR: 8271515: Integration of JEP 417: Vector API (Third Incubator) [v3]
On Tue, 19 Oct 2021 19:51:54 GMT, Paul Sandoz wrote:
>> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/ByteVector.java
>> line 603:
>>
>>> 601: if (opKind(op, VO_SPECIAL)) {
>>> 602: if (op == ZOMO) {
>>> 603: return blend(broadcast(-1), compare(NE, 0, m));
>>
>> This doesn't look correct. The lanes where mask is false should get the
>> original lane value in this vector.
>
> That should work, since `compare(NE, 0, m) === compare(NE, 0).and(m)`, so
> when an `m` lane is unset the lane element of `this` vector will be selected.
>
> Running jshell against a build of PR:
>
> $ ~/Projects/jdk/jdk/build/macosx-x86_64-server-release/images/jdk/bin/jshell
> --add-modules jdk.incubator.vector
> | Welcome to JShell -- Version 18-internal
> | For an introduction type: /help intro
>
> jshell> import jdk.incubator.vector.*
>
> jshell> var s = IntVector.SPECIES_256;
> s ==> Species[int, 8, S_256_BIT]
>
> jshell> var v = IntVector.fromArray(s, new int[]{0, 1, 0, -2, 0, 3, 0, -4},
> 0);
> v ==> [0, 1, 0, -2, 0, 3, 0, -4]
>
> jshell> var z = v.lanewise(VectorOperators.ZOMO);
> z ==> [0, -1, 0, -1, 0, -1, 0, -1]
>
> jshell> z = v.lanewise(VectorOperators.ZOMO, s.loadMask(new boolean[]{false,
> false, false, false, true, true, true, true}, 0));
> z ==> [0, 1, 0, -2, 0, -1, 0, -1]
>
> jshell>
Yes, you are correct. There is no problem here.
-
PR: https://git.openjdk.java.net/jdk/pull/5873
Re: RFR: 8275512: Upgrade required version of jtreg to 6.1 [v2]
On Tue, 19 Oct 2021 17:24:17 GMT, Weijun Wang wrote: >> As a follow up of JEP 411, we will soon disallow security manager by >> default. jtreg 6.1 does not set its own security manager if JDK version is >> >= 18. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > upgrade the version in GHA config > > only in patch2: > unchanged: Marked as reviewed by jjg (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6012
Re: RFR: 8275512: Upgrade required version of jtreg to 6.1 [v2]
On Tue, 19 Oct 2021 17:24:17 GMT, Weijun Wang wrote: >> As a follow up of JEP 411, we will soon disallow security manager by >> default. jtreg 6.1 does not set its own security manager if JDK version is >> >= 18. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > upgrade the version in GHA config > > only in patch2: > unchanged: Marked as reviewed by mchung (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6012
Integrated: 8275512: Upgrade required version of jtreg to 6.1
On Tue, 19 Oct 2021 13:51:45 GMT, Weijun Wang wrote: > As a follow up of JEP 411, we will soon disallow security manager by default. > jtreg 6.1 does not set its own security manager if JDK version is >= 18. This pull request has now been integrated. Changeset: c24fb852 Author:Weijun Wang URL: https://git.openjdk.java.net/jdk/commit/c24fb852f20bf0fc2817dfed52ff1609a5bced59 Stats: 8 lines in 7 files changed: 0 ins; 0 del; 8 mod 8275512: Upgrade required version of jtreg to 6.1 Reviewed-by: ihse, iignatyev, joehw, lancea, jjg, mchung - PR: https://git.openjdk.java.net/jdk/pull/6012
Re: RFR: 8272614: Unused parameters in MethodHandleNatives linking methods
On Tue, 19 Oct 2021 17:12:16 GMT, Harold Seigel wrote: > Please review this small fix for JDK-8272614 to remove the unused indexInCP > argument to linkCallSite() and linkDynamicConstant(). The fix was tested > with Mach5 tiers 1-2 on Linux, Mac OS, and Windows, and Mach5 tiers 3-6 on > Linux x64. > > Thanks, Harold LGTM! Thanks, David - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6021
Re: RFR: 8271515: Integration of JEP 417: Vector API (Third Incubator) [v4]
> This PR improves the performance of vector operations that accept masks on > architectures that support masking in hardware, specifically Intel AVX512 and > ARM SVE. > > On architectures that do not support masking in hardware the same technique > as before is applied to most operations, specifically composition using blend. > > Masked loads/stores are a special form of masked operation that require > additional care to ensure out-of-bounds access throw exceptions. The range > checking has not been fully optimized and will require further work. > > No API enhancements were required and only a few additional tests were needed. Paul Sandoz has updated the pull request incrementally with one additional commit since the last revision: Resolve review comments. - Changes: - all: https://git.openjdk.java.net/jdk/pull/5873/files - new: https://git.openjdk.java.net/jdk/pull/5873/files/32d58f17..101cd23a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5873&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5873&range=02-03 Stats: 50 lines in 4 files changed: 1 ins; 49 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5873.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5873/head:pull/5873 PR: https://git.openjdk.java.net/jdk/pull/5873
Re: RFR: 8271515: Integration of JEP 417: Vector API (Third Incubator) [v4]
On Tue, 19 Oct 2021 22:37:10 GMT, Paul Sandoz wrote: >> This PR improves the performance of vector operations that accept masks on >> architectures that support masking in hardware, specifically Intel AVX512 >> and ARM SVE. >> >> On architectures that do not support masking in hardware the same technique >> as before is applied to most operations, specifically composition using >> blend. >> >> Masked loads/stores are a special form of masked operation that require >> additional care to ensure out-of-bounds access throw exceptions. The range >> checking has not been fully optimized and will require further work. >> >> No API enhancements were required and only a few additional tests were >> needed. > > Paul Sandoz has updated the pull request incrementally with one additional > commit since the last revision: > > Resolve review comments. Marked as reviewed by sviswanathan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5873
Re: RFR: 8271515: Integration of JEP 417: Vector API (Third Incubator) [v3]
On Tue, 19 Oct 2021 21:14:08 GMT, Sandhya Viswanathan wrote: >> Paul Sandoz has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains seven commits: >> >> - Merge branch 'master' into JDK-8271515-vector-api >> - Apply patch from https://github.com/openjdk/panama-vector/pull/152 >> - Apply patch from https://github.com/openjdk/panama-vector/pull/142 >> - Apply patch from https://github.com/openjdk/panama-vector/pull/139 >> - Apply patch from https://github.com/openjdk/panama-vector/pull/151 >> - Add new files. >> - 8271515: Integration of JEP 417: Vector API (Third Incubator) > > src/jdk.incubator.vector/share/classes/jdk/incubator/vector/VectorMask.java > line 574: > >> 572: * @throws ClassCastException if the species is wrong >> 573: */ >> 574: abstract VectorMask check(Class> >> maskClass, Vector vector); > > This is a package-private method so the java doc style comments are not > needed here. I think that is fine, documentation for us :-) I converted to package private since it only really makes sense for internal use right now. - PR: https://git.openjdk.java.net/jdk/pull/5873
Re: RFR: 8271515: Integration of JEP 417: Vector API (Third Incubator) [v3]
On Sat, 16 Oct 2021 00:56:14 GMT, Paul Sandoz wrote: >> This PR improves the performance of vector operations that accept masks on >> architectures that support masking in hardware, specifically Intel AVX512 >> and ARM SVE. >> >> On architectures that do not support masking in hardware the same technique >> as before is applied to most operations, specifically composition using >> blend. >> >> Masked loads/stores are a special form of masked operation that require >> additional care to ensure out-of-bounds access throw exceptions. The range >> checking has not been fully optimized and will require further work. >> >> No API enhancements were required and only a few additional tests were >> needed. > > Paul Sandoz has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains seven commits: > > - Merge branch 'master' into JDK-8271515-vector-api > - Apply patch from https://github.com/openjdk/panama-vector/pull/152 > - Apply patch from https://github.com/openjdk/panama-vector/pull/142 > - Apply patch from https://github.com/openjdk/panama-vector/pull/139 > - Apply patch from https://github.com/openjdk/panama-vector/pull/151 > - Add new files. > - 8271515: Integration of JEP 417: Vector API (Third Incubator) The Java changes look good to me. src/jdk.incubator.vector/share/classes/jdk/incubator/vector/VectorMask.java line 574: > 572: * @throws ClassCastException if the species is wrong > 573: */ > 574: abstract VectorMask check(Class> > maskClass, Vector vector); This is a package-private method so the java doc style comments are not needed here. - PR: https://git.openjdk.java.net/jdk/pull/5873
Re: RFR: 8271515: Integration of JEP 417: Vector API (Third Incubator) [v3]
On Tue, 19 Oct 2021 22:34:13 GMT, Paul Sandoz wrote: >> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/VectorMask.java >> line 574: >> >>> 572: * @throws ClassCastException if the species is wrong >>> 573: */ >>> 574: abstract VectorMask check(Class> >>> maskClass, Vector vector); >> >> This is a package-private method so the java doc style comments are not >> needed here. > > I think that is fine, documentation for us :-) I converted to package private > since it only really makes sense for internal use right now. Sounds good. - PR: https://git.openjdk.java.net/jdk/pull/5873
Re: RFR: 8153490: Cannot setBytes() if incoming buffer's length is bigger than number of elements we want to insert. [v5]
On Tue, 19 Oct 2021 06:32:57 GMT, Mitsuru Kariya wrote: > The pre-submit test seems to have failed because the compiler was not found > in some environments. > Should I take any action? > Or should I issue the /integrate pull request command? You should be OK. Just as an extra sanity check, I will run those tiers internally tomorrow so please stay tuned - PR: https://git.openjdk.java.net/jdk/pull/4001
Re: RFR: 8271515: Integration of JEP 417: Vector API (Third Incubator) [v5]
> This PR improves the performance of vector operations that accept masks on > architectures that support masking in hardware, specifically Intel AVX512 and > ARM SVE. > > On architectures that do not support masking in hardware the same technique > as before is applied to most operations, specifically composition using blend. > > Masked loads/stores are a special form of masked operation that require > additional care to ensure out-of-bounds access throw exceptions. The range > checking has not been fully optimized and will require further work. > > No API enhancements were required and only a few additional tests were needed. Paul Sandoz has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains ten commits: - Merge branch 'master' into JDK-8271515-vector-api - Resolve review comments. - Merge branch 'master' into JDK-8271515-vector-api - Apply patch from https://github.com/openjdk/panama-vector/pull/152 - Apply patch from https://github.com/openjdk/panama-vector/pull/142 - Apply patch from https://github.com/openjdk/panama-vector/pull/139 - Apply patch from https://github.com/openjdk/panama-vector/pull/151 - Add new files. - 8271515: Integration of JEP 417: Vector API (Third Incubator) - Changes: https://git.openjdk.java.net/jdk/pull/5873/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5873&range=04 Stats: 21956 lines in 105 files changed: 16182 ins; 2080 del; 3694 mod Patch: https://git.openjdk.java.net/jdk/pull/5873.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5873/head:pull/5873 PR: https://git.openjdk.java.net/jdk/pull/5873
Re: RFR: 8271515: Integration of JEP 417: Vector API (Third Incubator) [v5]
On Wed, 20 Oct 2021 00:22:30 GMT, Paul Sandoz wrote:
>> This PR improves the performance of vector operations that accept masks on
>> architectures that support masking in hardware, specifically Intel AVX512
>> and ARM SVE.
>>
>> On architectures that do not support masking in hardware the same technique
>> as before is applied to most operations, specifically composition using
>> blend.
>>
>> Masked loads/stores are a special form of masked operation that require
>> additional care to ensure out-of-bounds access throw exceptions. The range
>> checking has not been fully optimized and will require further work.
>>
>> No API enhancements were required and only a few additional tests were
>> needed.
>
> Paul Sandoz has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains ten commits:
>
> - Merge branch 'master' into JDK-8271515-vector-api
> - Resolve review comments.
> - Merge branch 'master' into JDK-8271515-vector-api
> - Apply patch from https://github.com/openjdk/panama-vector/pull/152
> - Apply patch from https://github.com/openjdk/panama-vector/pull/142
> - Apply patch from https://github.com/openjdk/panama-vector/pull/139
> - Apply patch from https://github.com/openjdk/panama-vector/pull/151
> - Add new files.
> - 8271515: Integration of JEP 417: Vector API (Third Incubator)
AArch64 changes look ok apart from some minor comments.
src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 2057:
> 2055:
> 2056: const int num_of_regs = PRegisterImpl::number_of_saved_registers;
> 2057: unsigned char regs[num_of_regs];
Seems clearer to use `PRegisterImpl::number_of_saved_registers` directly rather
than introducing `num_of_regs`.
src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 2078:
> 2076: }
> 2077:
> 2078: // Return the number of dwords poped
popped
src/hotspot/cpu/aarch64/register_aarch64.hpp line 248:
> 246: // AArch64 has 8 governing predicate registers, but p7 is used as an
> 247: // all-1s register so the predicates to save are from p0 to p6 if we
> 248: // don't have non-governing predicate registers support.
This comment is a bit difficult to read. How about:
// AArch64 has 8 governing predicate registers, but p7 is used as an
// all-1s register so the predicates to save are from p0 to p6. We
// don't support non-governing predicate registers.
src/hotspot/cpu/aarch64/vmreg_aarch64.hpp line 42:
> 40:
> 41: inline Register as_Register() {
> 42: assert(is_Register(), "must be");
I think it's better to leave this file as it was if you're only making
whitespace changes here.
-
Marked as reviewed by ngasson (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/5873
