Re: RFR: 8280010: Remove double buffering of InputStream for Properties.load
On Mon, 10 Jan 2022 20:46:36 GMT, Andrey Turbanov wrote: > `Properties.load` uses `java.util.Properties.LineReader`. LineReader already > buffers input stream. Hence wrapping InputStream in BufferedInputStream is > redundant. Marked as reviewed by serb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7021
RFR: 8279508: Auto-vectorize Math.round API
Summary of changes: - Intrinsify Math.round(float) and Math.round(double) APIs. - Extend auto-vectorizer to infer vector operations on encountering scalar IR nodes for above intrinsics. - Test creation using new IR testing framework. Following are the performance number of a JMH micro included with the patch Test System: Intel(R) Xeon(R) Platinum 8380 CPU @ 2.30GHz (Icelake Server) | | BASELINE AVX2 | WithOpt AVX2 | Gain (opt/baseline) | Baseline AVX3 | Withopt AVX3 | Gain (opt/baseline) -- | -- | -- | -- | -- | -- | -- | -- Benchmark | ARRAYLEN | Score (ops/ms) | Score (ops/ms) | | Score (ops/ms) | Score (ops/ms) | FpRoundingBenchmark.test_round_double | 1024 | 518.532 | 1364.066 | 2.630630318 | 512.908 | 4292.11 | 8.368186887 FpRoundingBenchmark.test_round_double | 2048 | 270.137 | 830.986 | 3.076165057 | 273.159 | 2459.116 | 9.002507697 FpRoundingBenchmark.test_round_float | 1024 | 752.436 | 7780.905 | 10.34095259 | 752.49 | 9506.694 | 12.63364829 FpRoundingBenchmark.test_round_float | 2048 | 389.499 | 4113.046 | 10.55983712 | 389.63 | 4863.673 | 12.48279907 Kindly review and share your feedback. Best Regards, Jatin - Commit messages: - 8279508: Auto-vectorize Math.round API Changes: https://git.openjdk.java.net/jdk/pull/7094/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7094=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8279508 Stats: 409 lines in 22 files changed: 342 ins; 1 del; 66 mod Patch: https://git.openjdk.java.net/jdk/pull/7094.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7094/head:pull/7094 PR: https://git.openjdk.java.net/jdk/pull/7094
Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos
On Fri, 14 Jan 2022 10:18:50 GMT, Daniel Fuchs wrote:
>> This is what was intended (equivalent)
>>
>> `if (s ==null || (s!="always" && s!="never" && !s.startsWith("domain")))`
>
> Argh - you're right I missed the fact that the 3 expressions where included
> in parenthesis. I read it as
>
> ! (s.equals("always")) || ...
Shall we log a message if the value is not one of the 3 forms?
-
PR: https://git.openjdk.java.net/jdk/pull/7065
Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos
On Fri, 14 Jan 2022 18:40:41 GMT, Michael McMahon wrote: >> src/java.base/share/classes/sun/net/www/http/HttpClient.java line 152: >> >>> 150: * If enabled (for a particular destination) then SPNEGO >>> authentication requests will include >>> 151: * a channel binding token for the destination server. The default >>> behavior and setting for the >>> 152: * property is "never" >> >> Maybe this description should be added to >> `src/java.base//share/classes/java/net/doc-files/net-properties.html` too? > > It's actually a purely system property rather than a Net property at the > moment (same as the other spnego ones). Maybe, I should convert them all to > net properties, so they can be documented/set in that file? This system property should only be used for TLS, and the CBT can be used in both the SPNEGO mechanism and the Kerberos 5 mechanism. Therefore I suggest the name should probably contain "tls" (or maybe "https") and "negotiate". BTW, will you reuse this system property if we decide to support CBT in NTLM as well? - PR: https://git.openjdk.java.net/jdk/pull/7065
Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos
On Fri, 14 Jan 2022 18:42:08 GMT, Michael McMahon wrote:
>> src/java.security.jgss/share/classes/module-info.java line 36:
>>
>>> 34: module java.security.jgss {
>>> 35: requires java.naming;
>>> 36: requires java.security.sasl;
>>
>> Someone from security-dev should probably review this and validate that this
>> is OK. I'm also a bit uncomfortable that we require a class from
>> `com.sun.jndi.ldap.sasl` even though `java.naming` is already required by
>> `java.security.jgss` - so maybe this is OK.
>
> Yes. I would like the security team to validate this.
I suggest moving the `TlsChannelBinding` class into
`java.base/sun.security.util` since it's not only used by LDAP anymore. You
might need to modify the types of exceptions thrown in the class and move the 2
final strings to some other class inside `java.security.sasl`.
-
PR: https://git.openjdk.java.net/jdk/pull/7065
Re: RFR: 8272746: ZipFile can't open big file (NegativeArraySizeException) [v3]
On Fri, 14 Jan 2022 11:05:06 GMT, Masanori Yano wrote: >> Could you please review the JDK-8272746 bug fixes? >> Since the array index is of type int, the overflow occurs when the value of >> end.cenlen is too large because of too many entries. >> It is necessary to read a part of the CEN from the file to fix the problem >> fundamentally, but the way will require an extensive fix and degrade >> performance. >> In practical terms, the size of the central directory rarely grows that >> large. So, I fixed it to check the size of the central directory and throw >> an exception if it is too large. > > Masanori Yano has updated the pull request incrementally with one additional > commit since the last revision: > > 8272746: ZipFile can't open big file (NegativeArraySizeException) Thank you for making the changes. Overall it looks much better Please add comments describing your constants as well as a comment describing the intent of your setup and test methods - PR: https://git.openjdk.java.net/jdk/pull/6927
Re: RFR: 8280041: Retry loop issues in java.io.ClassCache
On Fri, 14 Jan 2022 20:50:24 GMT, Roman Kennke wrote: > One additional improvement would be to change r.get() == null to > r.refersTo(null) to check for cleared reference, that avoids reviving the > referent, e.g. in SATB GCs. I leave that up to you. I think that does not work, because we want to strongly reference the `val` for eventual return. - PR: https://git.openjdk.java.net/jdk/pull/7092
Re: RFR: 8280041: Retry loop issues in java.io.ClassCache
On Fri, 14 Jan 2022 19:27:18 GMT, Aleksey Shipilev wrote: > JDK-8277072 introduced java.io.ClassCache, but there seem to be at least two > issues with it: > - The cache cannot disambiguate between cleared SoftReference and the > accidental passing of `null` value; in that case, the retry loop would spin > indefinitely; > - If retry loop would spin several times, every time discovering a cleared > SoftReference, it would create and register new SoftReference on the > ReferenceQueue. However, it would not *drain* the RQ in the loop; in that > case, we might have the unbounded garbage accumulating in RQ; > > Attention @rkennke, @plevart. > > Additional testing: > - [x] Linux x86_64 fastdebug `java/io/ObjectStreamClass` > - [x] Linux x86_64 fastdebug `tier1` > - [ ] Linux x86_64 fastdebug `tier2` > - [ ] Linux x86_64 fastdebug `tier3` Right, good catches! It seems a bit weird that the constructors of Reference would accept null referents anyway, but it is like it is. *shrugs* One additional improvement would be to change r.get() == null to r.refersTo(null) to check for cleared reference, that avoids reviving the referent, e.g. in SATB GCs. I leave that up to you. - Marked as reviewed by rkennke (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7092
RFR: 8280041: Retry loop issues in java.io.ClassCache
JDK-8277072 introduced java.io.ClassCache, but there seem to be at least two issues with it: - The cache cannot disambiguate between cleared SoftReference and the accidental passing of `null` value; in that case, the retry loop would spin indefinitely; - If retry loop would spin several times, every time discovering a cleared SoftReference, it would create and register new SoftReference on the ReferenceQueue. However, it would not *drain* the RQ in the loop; in that case, we might have the unbounded garbage accumulating in RQ; Attention @rkennke, @plevart. Additional testing: - [x] Linux x86_64 fastdebug `java/io/ObjectStreamClass` - [x] Linux x86_64 fastdebug `tier1` - [ ] Linux x86_64 fastdebug `tier2` - [ ] Linux x86_64 fastdebug `tier3` - Commit messages: - Fix Changes: https://git.openjdk.java.net/jdk/pull/7092/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7092=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8280041 Stats: 6 lines in 1 file changed: 4 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7092.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7092/head:pull/7092 PR: https://git.openjdk.java.net/jdk/pull/7092
Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos
On Fri, 14 Jan 2022 14:52:13 GMT, Daniel Fuchs wrote:
>> Hi,
>>
>> This change adds Channel Binding Token (CBT) support to HTTPS
>> (java.net.HttpsURLConnection) when used with the Negotiate (SPNEGO,
>> Kerberos) authentication scheme. When enabled, the implementation
>> preemptively includes a CBT with authentication requests over Kerberos. The
>> feature is enabled as follows:
>>
>> A system property "jdk.spnego.cbt" is defined which can have the values
>> "never" (default), which means the feature is disabled, "always", which
>> means the CBT is included for all https Negotiate authentications, or it can
>> take the form "domain:a,b.c,*.d.com" which is a comma separated list of
>> domains/hosts where the feature is enabled, and disabled everywhere else. In
>> the given example, the CBT would be included in authentication requests for
>> hosts "a", "b.c" and all hosts under the domain "d.com" and all of its
>> sub-domains.
>>
>> A test will be added separately to the implementation.
>>
>> Bug report: https://bugs.openjdk.java.net/browse/JDK-8279842
>>
>> Thanks,
>> Michael
>
> src/java.base/share/classes/sun/net/www/http/HttpClient.java line 152:
>
>> 150: * If enabled (for a particular destination) then SPNEGO
>> authentication requests will include
>> 151: * a channel binding token for the destination server. The default
>> behavior and setting for the
>> 152: * property is "never"
>
> Maybe this description should be added to
> `src/java.base//share/classes/java/net/doc-files/net-properties.html` too?
It's actually a purely system property rather than a Net property at the moment
(same as the other spnego ones). Maybe, I should convert them all to net
properties, so they can be documented/set in that file?
> src/java.security.jgss/share/classes/module-info.java line 36:
>
>> 34: module java.security.jgss {
>> 35: requires java.naming;
>> 36: requires java.security.sasl;
>
> Someone from security-dev should probably review this and validate that this
> is OK. I'm also a bit uncomfortable that we require a class from
> `com.sun.jndi.ldap.sasl` even though `java.naming` is already required by
> `java.security.jgss` - so maybe this is OK.
Yes. I would like the security team to validate this.
-
PR: https://git.openjdk.java.net/jdk/pull/7065
Re: RFR: JDK-8277175 : Add a parallel multiply method to BigInteger [v7]
On Fri, 14 Jan 2022 09:15:37 GMT, kabutz wrote: >>> > embarrassingly parallelizable >>> >>> Having looked at [embarrassingly >>> parallel](https://en.wikipedia.org/wiki/Embarrassingly_parallel), I'm not >>> certain that this particular problem would qualify. The algorithm is easy >>> to parallelize, but in the end we still have some rather large numbers, so >>> memory will be our primary dominator. I'd expect to see a linear speedup if >>> it was "perfectly parallel", but this does not come close to that. >> >> Ok, fair point, to avoid possible confusion I have removed "embarrassingly". >> I don't think we need to refer to other algorithms. > > Hi @PaulSandoz is there anything else that we need to do? Or is this in the > hopper for Java 19 already? @kabutz i just finalized the CSR (the new year break meant all this is taking longer than usual). Based on that review we may need to make some tweaks to the API doc, which should be quick to process. So it should be in the bag for 19. - PR: https://git.openjdk.java.net/jdk/pull/6409
Re: RFR: 8280035: Use Class.isInstance instead of Class.isAssignableFrom where applicable
On Thu, 13 Jan 2022 08:25:22 GMT, Andrey Turbanov wrote:
> Method `Class.isAssignableFrom` is often used in form of:
>
> if (clazz.isAssignableFrom(obj.getClass())) {
> Such condition could be simplified to more shorter and performarnt code
>
> if (clazz.isInstance(obj)) {
>
> Replacement is equivalent if it's known that `obj != null`.
> In JDK codebase there are many such places.
>
> I tried to measure performance via JMH
>
> Class integerClass = Integer.class;
> Class numberClass = Number.class;
>
> Object integerObject = 45666;
> Object doubleObject = 8777d;
>
> @Benchmark
> public boolean integerInteger_isInstance() {
> return integerClass.isInstance(integerObject);
> }
>
> @Benchmark
> public boolean integerInteger_isAssignableFrom() {
> return integerClass.isAssignableFrom(integerObject.getClass());
> }
>
> @Benchmark
> public boolean integerDouble_isInstance() {
> return integerClass.isInstance(doubleObject);
> }
>
> @Benchmark
> public boolean integerDouble_isAssignableFrom() {
> return integerClass.isAssignableFrom(doubleObject.getClass());
> }
>
> @Benchmark
> public boolean numberDouble_isInstance() {
> return numberClass.isInstance(doubleObject);
> }
>
> @Benchmark
> public boolean numberDouble_isAssignableFrom() {
> return numberClass.isAssignableFrom(doubleObject.getClass());
> }
>
> @Benchmark
> public boolean numberInteger_isInstance() {
> return numberClass.isInstance(integerObject);
> }
>
> @Benchmark
> public boolean numberInteger_isAssignableFrom() {
> return numberClass.isAssignableFrom(integerObject.getClass());
> }
>
> @Benchmark
> public void numberIntegerDouble_isInstance(Blackhole bh) {
> bh.consume(numberClass.isInstance(integerObject));
> bh.consume(numberClass.isInstance(doubleObject));
> }
>
> @Benchmark
> public void integerIntegerDouble_isAssignableFrom(Blackhole bh) {
> bh.consume(integerClass.isAssignableFrom(integerObject.getClass()));
> bh.consume(integerClass.isAssignableFrom(doubleObject.getClass()));
> }
>
> `isInstance` is a bit faster than `isAssignableFrom`
>
> Benchmark Mode Cnt Score Error Units
> integerDouble_isAssignableFrom avgt5 1,173 ± 0,026 ns/op
> integerDouble_isInstance avgt5 0,939 ± 0,038 ns/op
> integerIntegerDouble_isAssignableFrom avgt5 2,106 ± 0,068 ns/op
> numberIntegerDouble_isInstance avgt5 1,516 ± 0,046 ns/op
> integerInteger_isAssignableFromavgt5 1,175 ± 0,029 ns/op
> integerInteger_isInstance avgt5 0,886 ± 0,017 ns/op
> numberDouble_isAssignableFrom avgt5 1,172 ± 0,007 ns/op
> numberDouble_isInstanceavgt5 0,891 ± 0,030 ns/op
> numberInteger_isAssignableFrom avgt5 1,169 ± 0,014 ns/op
> numberInteger_isInstance avgt5 0,887 ± 0,016 ns/op
src/jdk.jfr/share/classes/jdk/jfr/internal/TypeLibrary.java line 405:
> 403: Object res = m.invoke(a, new Object[0]);
> 404: if (res instanceof Annotation[]) {
> 405: for (Annotation rep : (Annotation[])
> m.invoke(a, new Object[0])) {
BTW it looks suspicious to have `m.invoke(a, new Object[0])` called again here.
Shouldn't it just reuse `res` variable instead?
-
PR: https://git.openjdk.java.net/jdk/pull/7061
RFR: 8280035: Use Class.isInstance instead of Class.isAssignableFrom where applicable
Method `Class.isAssignableFrom` is often used in form of:
if (clazz.isAssignableFrom(obj.getClass())) {
Such condition could be simplified to more shorter and performarnt code
if (clazz.isInstance(obj)) {
Replacement is equivalent if it's known that `obj != null`.
In JDK codebase there are many such places.
I tried to measure performance via JMH
Class integerClass = Integer.class;
Class numberClass = Number.class;
Object integerObject = 45666;
Object doubleObject = 8777d;
@Benchmark
public boolean integerInteger_isInstance() {
return integerClass.isInstance(integerObject);
}
@Benchmark
public boolean integerInteger_isAssignableFrom() {
return integerClass.isAssignableFrom(integerObject.getClass());
}
@Benchmark
public boolean integerDouble_isInstance() {
return integerClass.isInstance(doubleObject);
}
@Benchmark
public boolean integerDouble_isAssignableFrom() {
return integerClass.isAssignableFrom(doubleObject.getClass());
}
@Benchmark
public boolean numberDouble_isInstance() {
return numberClass.isInstance(doubleObject);
}
@Benchmark
public boolean numberDouble_isAssignableFrom() {
return numberClass.isAssignableFrom(doubleObject.getClass());
}
@Benchmark
public boolean numberInteger_isInstance() {
return numberClass.isInstance(integerObject);
}
@Benchmark
public boolean numberInteger_isAssignableFrom() {
return numberClass.isAssignableFrom(integerObject.getClass());
}
@Benchmark
public void numberIntegerDouble_isInstance(Blackhole bh) {
bh.consume(numberClass.isInstance(integerObject));
bh.consume(numberClass.isInstance(doubleObject));
}
@Benchmark
public void integerIntegerDouble_isAssignableFrom(Blackhole bh) {
bh.consume(integerClass.isAssignableFrom(integerObject.getClass()));
bh.consume(integerClass.isAssignableFrom(doubleObject.getClass()));
}
`isInstance` is a bit faster than `isAssignableFrom`
Benchmark Mode Cnt Score Error Units
integerDouble_isAssignableFrom avgt5 1,173 ± 0,026 ns/op
integerDouble_isInstance avgt5 0,939 ± 0,038 ns/op
integerIntegerDouble_isAssignableFrom avgt5 2,106 ± 0,068 ns/op
numberIntegerDouble_isInstance avgt5 1,516 ± 0,046 ns/op
integerInteger_isAssignableFromavgt5 1,175 ± 0,029 ns/op
integerInteger_isInstance avgt5 0,886 ± 0,017 ns/op
numberDouble_isAssignableFrom avgt5 1,172 ± 0,007 ns/op
numberDouble_isInstanceavgt5 0,891 ± 0,030 ns/op
numberInteger_isAssignableFrom avgt5 1,169 ± 0,014 ns/op
numberInteger_isInstance avgt5 0,887 ± 0,016 ns/op
-
Commit messages:
- [PATCH] Use Class.isInstance instead of Class.isAssignableFrom where
applicable
Changes: https://git.openjdk.java.net/jdk/pull/7061/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7061=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8280035
Stats: 25 lines in 10 files changed: 0 ins; 3 del; 22 mod
Patch: https://git.openjdk.java.net/jdk/pull/7061.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/7061/head:pull/7061
PR: https://git.openjdk.java.net/jdk/pull/7061
Re: RFR: 8280010: Remove double buffering of InputStream for Properties.load
On Mon, 10 Jan 2022 20:46:36 GMT, Andrey Turbanov wrote: > `Properties.load` uses `java.util.Properties.LineReader`. LineReader already > buffers input stream. Hence wrapping InputStream in BufferedInputStream is > redundant. Marked as reviewed by sspitsyn (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7021
Integrated: 8279918: Fix various doc typos
On Thu, 13 Jan 2022 10:30:07 GMT, Pavel Rappo wrote: > - Most of the typos are of a trivial kind: missing whitespace. > - If any of the typos should be fixed in the upstream projects instead, > please say so; I will drop those typos from the patch. > - As I understand it, ` ` in ImageInputStream and DataInput is an irrelevant > formatting artefact and thus can be removed. > - `` is an apostrophe, which does not require to be encoded. This pull request has now been integrated. Changeset: f1805309 Author:Pavel Rappo URL: https://git.openjdk.java.net/jdk/commit/f1805309352a22119ae2edf8bfbb596f00936224 Stats: 88 lines in 35 files changed: 0 ins; 0 del; 88 mod 8279918: Fix various doc typos Reviewed-by: kevinw, lancea, mullan, sspitsyn, naoto, jlahoda, azvegint, egahlin, jjg - PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos [v2]
On Thu, 13 Jan 2022 14:01:04 GMT, Pavel Rappo wrote: >> - Most of the typos are of a trivial kind: missing whitespace. >> - If any of the typos should be fixed in the upstream projects instead, >> please say so; I will drop those typos from the patch. >> - As I understand it, ` ` in ImageInputStream and DataInput is an irrelevant >> formatting artefact and thus can be removed. >> - `` is an apostrophe, which does not require to be encoded. > > Pavel Rappo has updated the pull request incrementally with one additional > commit since the last revision: > > Additional typos jdk.compiler and jdk.javadoc look good - Marked as reviewed by jjg (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: JDK-8279954 - java/lang/StringBuffer(StringBuilder)/HugeCapacity.java intermittently fails
On Fri, 14 Jan 2022 14:33:13 GMT, Jim Laskey wrote: > Tests were fatally failing (windows) on Github actions. Pumping up the memory > requirements will hopefully alleviate. "Perhaps one of the systems used to run GHA has an inconsistently sized page file" That has been my thought all along. I pinged the gc team and infra about this. Nothing from infra but DH thought that the @requires might work round the issue. - PR: https://git.openjdk.java.net/jdk/pull/7086
Re: RFR: JDK-8277520: Implement JDK-8 default methods for IdentityHashMap [v2]
On Thu, 16 Dec 2021 20:48:39 GMT, liach wrote: >> Might need a CSR as now `computeIfAbsent` `computeIfPresent` `compute` >> `merge` would throw CME if the functions modified the map itself, and there >> are corresponding specification changes. > > liach 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 five additional commits since the > last revision: > > - Merge branch 'master' of https://git.openjdk.java.net/jdk into > identityhashmap-default > - update dates > - Also test cme for identity hash map > - Fix putIfAbsent > - JDK-8277520: Implement JDK-8 default methods for IdentityHashMap The PR description doesn't do the change request justice. At least the first paragraph from the bug report would be useful. The rationale seems ok but not compelling: "may save a few hash table lookups and moderately boost the performance of IdentityHashMap". Please quantify the improvement with JMH test results. There may be an existing test in `test/micro/org/openjdk/bench/java/util` to build on. Thanks - PR: https://git.openjdk.java.net/jdk/pull/6532
Re: RFR: JDK-8279954 - java/lang/StringBuffer(StringBuilder)/HugeCapacity.java intermittently fails
On Fri, 14 Jan 2022 14:33:13 GMT, Jim Laskey wrote: > Tests were fatally failing (windows) on Github actions. Pumping up the memory > requirements will hopefully alleviate. If the error message: ```OpenJDK 64-Bit Server VM warning: INFO: os::commit_memory(0x01cfdf61, 10485760, 0) failed; error='The paging file is too small for this operation to complete' (DOS error/errno=1455) ``` Correctly reflects the error, raising the requested heap size won't help. The size of the paging file isn't sufficient. Without knowing how the system configuration its hard to come to a conclusion. Perhaps one of the systems used to run GHA has an inconsistently sized page file. $.02 - PR: https://git.openjdk.java.net/jdk/pull/7086
Integrated: 8278851: Correct signer logic for jars signed with multiple digestalgs
On Wed, 12 Jan 2022 21:57:22 GMT, Sean Mullan wrote: > If a JAR is signed with multiple digest algorithms and one of the digest > algorithms is disabled, `ManifestEntryVerifier.verify()` was incorrectly > returning null indicating that the jar entry has no signers. > > This fixes the issue such that an entry is considered signed if at least one > of the digest algorithms is not disabled and the digest match passes. This > makes the fix consistent with how multiple digest algorithms are handled in > the Signature File. This also fixes an issue in the > `ManifestEntryVerifier.getParams()` method in which it was incorrectly > checking the algorithm constraints against all signers of a JAR when it > should check them only against the signers of the entry that is being > verified. > > An additional cache has also been added to avoid checking if the digest > algorithm is disabled more than once for entries signed by the same set of > signers. This pull request has now been integrated. Changeset: 61b89443 Author:Sean Mullan URL: https://git.openjdk.java.net/jdk/commit/61b8944327e3d12cf58dc3f6bc45ecbeba4ef611 Stats: 264 lines in 3 files changed: 214 ins; 20 del; 30 mod 8278851: Correct signer logic for jars signed with multiple digestalgs Reviewed-by: coffeys, weijun - PR: https://git.openjdk.java.net/jdk/pull/7056
Re: RFR: JDK-8279954 - java/lang/StringBuffer(StringBuilder)/HugeCapacity.java intermittently fails
On Fri, 14 Jan 2022 14:33:13 GMT, Jim Laskey wrote: > Tests were fatally failing (windows) on Github actions. Pumping up the memory > requirements will hopefully alleviate. Looks fine. - Marked as reviewed by shade (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7086
Re: RFR: 8278851: Correct signer logic for jars signed with multiple digestalgs [v2]
On Thu, 13 Jan 2022 21:57:57 GMT, Sean Mullan wrote: >> If a JAR is signed with multiple digest algorithms and one of the digest >> algorithms is disabled, `ManifestEntryVerifier.verify()` was incorrectly >> returning null indicating that the jar entry has no signers. >> >> This fixes the issue such that an entry is considered signed if at least one >> of the digest algorithms is not disabled and the digest match passes. This >> makes the fix consistent with how multiple digest algorithms are handled in >> the Signature File. This also fixes an issue in the >> `ManifestEntryVerifier.getParams()` method in which it was incorrectly >> checking the algorithm constraints against all signers of a JAR when it >> should check them only against the signers of the entry that is being >> verified. >> >> An additional cache has also been added to avoid checking if the digest >> algorithm is disabled more than once for entries signed by the same set of >> signers. > > Sean Mullan has updated the pull request incrementally with one additional > commit since the last revision: > > Change permittedAlgs variable name. > Move put methods out of checkConstraints(). Each `CodeSigner[]` reference uniquely represents the signers of an entry. Multiple entries can map to the same `CodeSigner[]` reference. We only need reference equality for the keys as each JAR entry is already mapped to an array of `CodeSigner`. It's basically an `IdentityHashMap` but we don't need to specifically use that. - PR: https://git.openjdk.java.net/jdk/pull/7056
Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos
On Thu, 13 Jan 2022 12:10:11 GMT, Michael McMahon wrote:
> Hi,
>
> This change adds Channel Binding Token (CBT) support to HTTPS
> (java.net.HttpsURLConnection) when used with the Negotiate (SPNEGO, Kerberos)
> authentication scheme. When enabled, the implementation preemptively includes
> a CBT with authentication requests over Kerberos. The feature is enabled as
> follows:
>
> A system property "jdk.spnego.cbt" is defined which can have the values
> "never" (default), which means the feature is disabled, "always", which means
> the CBT is included for all https Negotiate authentications, or it can take
> the form "domain:a,b.c,*.d.com" which is a comma separated list of
> domains/hosts where the feature is enabled, and disabled everywhere else. In
> the given example, the CBT would be included in authentication requests for
> hosts "a", "b.c" and all hosts under the domain "d.com" and all of its
> sub-domains.
>
> A test will be added separately to the implementation.
>
> Bug report: https://bugs.openjdk.java.net/browse/JDK-8279842
>
> Thanks,
> Michael
Have you been able to test this on a specific setup?
Would be good to hear from @msheppar too.
src/java.base/share/classes/sun/net/www/http/HttpClient.java line 152:
> 150: * If enabled (for a particular destination) then SPNEGO
> authentication requests will include
> 151: * a channel binding token for the destination server. The default
> behavior and setting for the
> 152: * property is "never"
Maybe this description should be added to
`src/java.base//share/classes/java/net/doc-files/net-properties.html` too?
src/java.security.jgss/share/classes/module-info.java line 36:
> 34: module java.security.jgss {
> 35: requires java.naming;
> 36: requires java.security.sasl;
Someone from security-dev should probably review this and validate that this is
OK. I'm also a bit uncomfortable that we require a class from
`com.sun.jndi.ldap.sasl` even though `java.naming` is already required by
`java.security.jgss` - so maybe this is OK.
-
PR: https://git.openjdk.java.net/jdk/pull/7065
Re: RFR: 8280010: Remove double buffering of InputStream for Properties.load
On Mon, 10 Jan 2022 20:46:36 GMT, Andrey Turbanov wrote: > `Properties.load` uses `java.util.Properties.LineReader`. LineReader already > buffers input stream. Hence wrapping InputStream in BufferedInputStream is > redundant. Changes to `java.util.logging` look fine. - PR: https://git.openjdk.java.net/jdk/pull/7021
RFR: JDK-8279954 - java/lang/StringBuffer(StringBuilder)/HugeCapacity.java intermittently fails
Tests were fatally failing (windows) on Github actions. Pumping up the memory requirements will hopefully alleviate. - Commit messages: - Bump up memory requirements. Changes: https://git.openjdk.java.net/jdk/pull/7086/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7086=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8279954 Stats: 5 lines in 2 files changed: 0 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/7086.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7086/head:pull/7086 PR: https://git.openjdk.java.net/jdk/pull/7086
Re: RFR: 8280010: Remove double buffering of InputStream for Properties.load
On Mon, 10 Jan 2022 20:46:36 GMT, Andrey Turbanov wrote: > `Properties.load` uses `java.util.Properties.LineReader`. LineReader already > buffers input stream. Hence wrapping InputStream in BufferedInputStream is > redundant. Marked as reviewed by amenkov (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7021
Re: RFR: 8279918: Fix various doc typos [v2]
On Thu, 13 Jan 2022 14:01:04 GMT, Pavel Rappo wrote: >> - Most of the typos are of a trivial kind: missing whitespace. >> - If any of the typos should be fixed in the upstream projects instead, >> please say so; I will drop those typos from the patch. >> - As I understand it, ` ` in ImageInputStream and DataInput is an irrelevant >> formatting artefact and thus can be removed. >> - `` is an apostrophe, which does not require to be encoded. > > Pavel Rappo has updated the pull request incrementally with one additional > commit since the last revision: > > Additional typos Marked as reviewed by egahlin (Reviewer). The JFR related changes looks OK. - PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8280010: Remove double buffering of InputStream for Properties.load
On Mon, 10 Jan 2022 20:46:36 GMT, Andrey Turbanov wrote:
> `Properties.load` uses `java.util.Properties.LineReader`. LineReader already
> buffers input stream. Hence wrapping InputStream in BufferedInputStream is
> redundant.
Checked. `BufferedInputStream` add a bit of overhead.
Benchmark
@BenchmarkMode(value = Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@Warmup(iterations = 6, time = 1, timeUnit = TimeUnit.SECONDS)
@Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
@Fork(1)
@State(Scope.Benchmark)
public class PropertiesLoad {
Properties properties;
private InputStream arrayInputStream;
private InputStream fileInputStream;
private Path filePath;
@Setup
public void setupStrings() throws IOException {
properties = new Properties();
String content = """
currentVersion=IdealGraphVisualizer {0}
LBL_splash_window_title=Starting IdealGraphVisualizer
SPLASH_WIDTH=475
SplashProgressBarBounds=0,273,475,6
SplashProgressBarColor=0xFF
SplashRunningTextBounds=10,283,460,12
SplashRunningTextColor=0xFF
""";
filePath = Files.createTempFile("benchmark", ".properties");
Files.writeString(filePath, content);
arrayInputStream = new
ByteArrayInputStream(content.getBytes(StandardCharsets.UTF_8));
fileInputStream = Files.newInputStream(filePath);
}
@TearDown
public void tearDown() throws IOException {
fileInputStream.close();
Files.delete(filePath);
}
@Benchmark
public Properties plain_ByteStream() throws IOException {
properties.load(arrayInputStream);
return properties;
}
@Benchmark
public Properties plain_fileStream() throws IOException {
properties.load(fileInputStream);
return properties;
}
@Benchmark
public Properties buffered_ByteStream() throws IOException {
properties.load(new BufferedInputStream(arrayInputStream));
return properties;
}
@Benchmark
public Properties buffered_fileStream() throws IOException {
properties.load(new BufferedInputStream(fileInputStream));
return properties;
}
public static void main(String[] args) throws RunnerException {
Options opt = new OptionsBuilder()
.include(PropertiesLoad.class.getSimpleName())
.build();
new Runner(opt).run();
}
}
Results:
Benchmark Mode Cnt Score Error Units
PropertiesLoad.buffered_ByteStream avgt5 2416,364 ± 46,209 ns/op
PropertiesLoad.buffered_fileStream avgt5 4276,015 ± 139,094 ns/op
PropertiesLoad.plain_ByteStream avgt5 1445,864 ± 649,779 ns/op
PropertiesLoad.plain_fileStream avgt5 3183,012 ± 198,974 ns/op
-
PR: https://git.openjdk.java.net/jdk/pull/7021
Re: RFR: 8280010: Remove double buffering of InputStream for Properties.load
On Mon, 10 Jan 2022 20:46:36 GMT, Andrey Turbanov wrote: > `Properties.load` uses `java.util.Properties.LineReader`. LineReader already > buffers input stream. Hence wrapping InputStream in BufferedInputStream is > redundant. The code change looks fine, but can you please check the actual performance impact, will the perf be the same after the change? - PR: https://git.openjdk.java.net/jdk/pull/7021
RFR: 8280010: Remove double buffering of InputStream for Properties.load
`Properties.load` uses `java.util.Properties.LineReader`. LineReader already buffers input stream. Hence wrapping InputStream in BufferedInputStream is redundant. - Commit messages: - [PATCH] Remove double buffering of InputStream for Properties.load - [PATCH] Remove double buffering of InputStream for Properties.load Changes: https://git.openjdk.java.net/jdk/pull/7021/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7021=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8280010 Stats: 30 lines in 8 files changed: 0 ins; 11 del; 19 mod Patch: https://git.openjdk.java.net/jdk/pull/7021.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7021/head:pull/7021 PR: https://git.openjdk.java.net/jdk/pull/7021
Re: RFR: 8279918: Fix various doc typos [v2]
On Thu, 13 Jan 2022 14:01:04 GMT, Pavel Rappo wrote: >> - Most of the typos are of a trivial kind: missing whitespace. >> - If any of the typos should be fixed in the upstream projects instead, >> please say so; I will drop those typos from the patch. >> - As I understand it, ` ` in ImageInputStream and DataInput is an irrelevant >> formatting artefact and thus can be removed. >> - `` is an apostrophe, which does not require to be encoded. > > Pavel Rappo has updated the pull request incrementally with one additional > commit since the last revision: > > Additional typos Client changes looks good to me. - Marked as reviewed by azvegint (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos [v2]
On Thu, 13 Jan 2022 14:01:04 GMT, Pavel Rappo wrote: >> - Most of the typos are of a trivial kind: missing whitespace. >> - If any of the typos should be fixed in the upstream projects instead, >> please say so; I will drop those typos from the patch. >> - As I understand it, ` ` in ImageInputStream and DataInput is an irrelevant >> formatting artefact and thus can be removed. >> - `` is an apostrophe, which does not require to be encoded. > > Pavel Rappo has updated the pull request incrementally with one additional > commit since the last revision: > > Additional typos javac changes look good to me. - Marked as reviewed by jlahoda (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7063
[jdk18] Integrated: 8279930: Synthetic cast causes generation of store barriers when using heap segments
On Wed, 12 Jan 2022 15:48:20 GMT, Maurizio Cimadamore wrote: > When looking at larger benchmarks, I noted a discrepancy between the > performance of off-heap segments and on-heap segments. Looking at the > assembly for the `MemorySegment::asSlice` method I could see some additional > barriers in the off-heap case, but could not initially make sense of them. > Vlad pointed me at G1 (in fact no such barrier was emitted when using a > different GC, such as the serial GC, or ZGC), and later Erik narrowed the > problem down to a failure in a C2 optimization to remove barriers around > initializing stores. This problem was caused by a synthetic cast added by > javac to a value (the base object) that initialized the newly created memory > segment slice. Because of that case, C2 missed the store as an "initializing" > one, and inserted additional barriers. This patch should make performance of > on-heap segments a lot more reliable, especially when slicing is involved. This pull request has now been integrated. Changeset: c6b02755 Author:Maurizio Cimadamore URL: https://git.openjdk.java.net/jdk18/commit/c6b027559c6e055b1475ada4001ef483b1a12d24 Stats: 292 lines in 3 files changed: 263 ins; 0 del; 29 mod 8279930: Synthetic cast causes generation of store barriers when using heap segments Reviewed-by: psandoz - PR: https://git.openjdk.java.net/jdk18/pull/97
Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos
On Thu, 13 Jan 2022 18:18:24 GMT, Daniel Fuchs wrote:
>> Hi,
>>
>> This change adds Channel Binding Token (CBT) support to HTTPS
>> (java.net.HttpsURLConnection) when used with the Negotiate (SPNEGO,
>> Kerberos) authentication scheme. When enabled, the implementation
>> preemptively includes a CBT with authentication requests over Kerberos. The
>> feature is enabled as follows:
>>
>> A system property "jdk.spnego.cbt" is defined which can have the values
>> "never" (default), which means the feature is disabled, "always", which
>> means the CBT is included for all https Negotiate authentications, or it can
>> take the form "domain:a,b.c,*.d.com" which is a comma separated list of
>> domains/hosts where the feature is enabled, and disabled everywhere else. In
>> the given example, the CBT would be included in authentication requests for
>> hosts "a", "b.c" and all hosts under the domain "d.com" and all of its
>> sub-domains.
>>
>> A test will be added separately to the implementation.
>>
>> Bug report: https://bugs.openjdk.java.net/browse/JDK-8279842
>>
>> Thanks,
>> Michael
>
> src/java.base/share/classes/sun/net/www/http/HttpClient.java line 180:
>
>> 178: static String normalizeCBT(String s) {
>> 179: if (s == null || ! (s.equals("always") ||
>> 180: s.equals("never") || s.startsWith("domain:"))) {
>
> I guess there's a `!` missing in front of `s.startsWith("domain:")` here?
This is what was intended (equivalent)
`if (s ==null || (s!="always" && s!="never" && !s.startsWith("domain")))`
-
PR: https://git.openjdk.java.net/jdk/pull/7065
Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos
On Fri, 14 Jan 2022 10:03:37 GMT, Michael McMahon wrote:
>> src/java.base/share/classes/sun/net/www/http/HttpClient.java line 180:
>>
>>> 178: static String normalizeCBT(String s) {
>>> 179: if (s == null || ! (s.equals("always") ||
>>> 180: s.equals("never") || s.startsWith("domain:"))) {
>>
>> I guess there's a `!` missing in front of `s.startsWith("domain:")` here?
>
> This is what was intended (equivalent)
>
> `if (s ==null || (s!="always" && s!="never" && !s.startsWith("domain")))`
Argh - you're right I missed the fact that the 3 expressions where included in
parenthesis. I read it as
! (s.equals("always")) || ...
-
PR: https://git.openjdk.java.net/jdk/pull/7065
Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos
On Thu, 13 Jan 2022 12:10:11 GMT, Michael McMahon wrote:
> Hi,
>
> This change adds Channel Binding Token (CBT) support to HTTPS
> (java.net.HttpsURLConnection) when used with the Negotiate (SPNEGO, Kerberos)
> authentication scheme. When enabled, the implementation preemptively includes
> a CBT with authentication requests over Kerberos. The feature is enabled as
> follows:
>
> A system property "jdk.spnego.cbt" is defined which can have the values
> "never" (default), which means the feature is disabled, "always", which means
> the CBT is included for all https Negotiate authentications, or it can take
> the form "domain:a,b.c,*.d.com" which is a comma separated list of
> domains/hosts where the feature is enabled, and disabled everywhere else. In
> the given example, the CBT would be included in authentication requests for
> hosts "a", "b.c" and all hosts under the domain "d.com" and all of its
> sub-domains.
>
> A test will be added separately to the implementation.
>
> Bug report: https://bugs.openjdk.java.net/browse/JDK-8279842
>
> Thanks,
> Michael
src/java.base/share/classes/sun/net/www/http/HttpClient.java line 180:
> 178: static String normalizeCBT(String s) {
> 179: if (s == null || ! (s.equals("always") ||
> 180: s.equals("never") || s.startsWith("domain:"))) {
I guess there's a `!` missing in front of `s.startsWith("domain:")` here?
-
PR: https://git.openjdk.java.net/jdk/pull/7065
RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos
Hi, This change adds Channel Binding Token (CBT) support to HTTPS (java.net.HttpsURLConnection) when used with the Negotiate (SPNEGO, Kerberos) authentication scheme. When enabled, the implementation preemptively includes a CBT with authentication requests over Kerberos. The feature is enabled as follows: A system property "jdk.spnego.cbt" is defined which can have the values "never" (default), which means the feature is disabled, "always", which means the CBT is included for all https Negotiate authentications, or it can take the form "domain:a,b.c,*.d.com" which is a comma separated list of domains/hosts where the feature is enabled, and disabled everywhere else. In the given example, the CBT would be included in authentication requests for hosts "a", "b.c" and all hosts under the domain "d.com" and all of its sub-domains. A test will be added separately to the implementation. Bug report: https://bugs.openjdk.java.net/browse/JDK-8279842 Thanks, Michael - Commit messages: - cleanup but still no test. Will be added in closed repo - First version of fix. No test and feature enabled always. Changes: https://git.openjdk.java.net/jdk/pull/7065/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7065=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8279842 Stats: 149 lines in 7 files changed: 143 ins; 0 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/7065.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7065/head:pull/7065 PR: https://git.openjdk.java.net/jdk/pull/7065
Re: RFR: 8272746: ZipFile can't open big file (NegativeArraySizeException) [v3]
> Could you please review the JDK-8272746 bug fixes? > Since the array index is of type int, the overflow occurs when the value of > end.cenlen is too large because of too many entries. > It is necessary to read a part of the CEN from the file to fix the problem > fundamentally, but the way will require an extensive fix and degrade > performance. > In practical terms, the size of the central directory rarely grows that > large. So, I fixed it to check the size of the central directory and throw an > exception if it is too large. Masanori Yano has updated the pull request incrementally with one additional commit since the last revision: 8272746: ZipFile can't open big file (NegativeArraySizeException) - Changes: - all: https://git.openjdk.java.net/jdk/pull/6927/files - new: https://git.openjdk.java.net/jdk/pull/6927/files/c54c50eb..621f1a68 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6927=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6927=01-02 Stats: 33 lines in 1 file changed: 10 ins; 9 del; 14 mod Patch: https://git.openjdk.java.net/jdk/pull/6927.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6927/head:pull/6927 PR: https://git.openjdk.java.net/jdk/pull/6927
Re: RFR: 8272746: ZipFile can't open big file (NegativeArraySizeException) [v2]
On Sat, 8 Jan 2022 16:09:59 GMT, Lance Andersen wrote: >> Masanori Yano has updated the pull request incrementally with one additional >> commit since the last revision: >> >> 8272746: ZipFile can't open big file (NegativeArraySizeException) > > Thank you for your work here. Have you also run your test using Apache > Commons? > > I have run this test over 2000 times via Mach5 on various hardware with some > runs taking over 12 minutes for the test to complete. So unless you can > refactor the test to be more efficient, we need to make the test a manual > test as this is more of a corner case. > > Please see the additional comments below @LanceAndersen Thank you for your detailed comments. I converted the test to use TestNG as you pointed out. Could you please review it again? - PR: https://git.openjdk.java.net/jdk/pull/6927
Re: RFR: JDK-8277175 : Add a parallel multiply method to BigInteger [v7]
On Thu, 16 Dec 2021 21:54:43 GMT, Paul Sandoz wrote: >>> > embarrassingly parallelizable >>> >>> Having looked at [embarrassingly >>> parallel](https://en.wikipedia.org/wiki/Embarrassingly_parallel), I'm not >>> certain that this particular problem would qualify. The algorithm is easy >>> to parallelize, but in the end we still have some rather large numbers, so >>> memory will be our primary dominator. I'd expect to see a linear speedup if >>> it was "perfectly parallel", but this does not come close to that. >> >> I ran fibonacci(100_000_000) with multiply() and parallelMultiply(). For >> multiply() we had: >> >> >> real 0m25.627s >> user 0m26.767s >> sys 0m1.197s >> >> >> and for parallelMultiply() we had >> >> >> real 0m10.030s >> user 1m2.205s >> sys 0m2.489s >> >> >> Thus we are 2.5 times faster on a 1-6-2 machine, but use more than 2x the >> user time. If it were perfectly parallel, shouldn't we expect to see the >> parallelMultiply() be close to user time of 26s and real time 4.3s? > >> > embarrassingly parallelizable >> >> Having looked at [embarrassingly >> parallel](https://en.wikipedia.org/wiki/Embarrassingly_parallel), I'm not >> certain that this particular problem would qualify. The algorithm is easy to >> parallelize, but in the end we still have some rather large numbers, so >> memory will be our primary dominator. I'd expect to see a linear speedup if >> it was "perfectly parallel", but this does not come close to that. > > Ok, fair point, to avoid possible confusion I have removed "embarrassingly". > I don't think we need to refer to other algorithms. Hi @PaulSandoz is there anything else that we need to do? Or is this in the hopper for Java 19 already? - PR: https://git.openjdk.java.net/jdk/pull/6409
