Re: RFR: 8299748: java/util/zip/Deinflate.java failing on s390x [v2]
On Mon, 6 Mar 2023 11:07:26 GMT, Jaikiran Pai wrote: >Now coming to this proposed patch, now that you are using local >ByteArrayOutputStream(s) for the deflate/inflate part in this check() method, >I think the out1 and out2 should no longer be needed in this method and thus >the method signature can be changed to remove these params. I see that this is >the only place where this method is used, so changing the method signature of >check() should be OK and shouldn't impact other parts of the test. @jaikiran I've removed these parameters, not sure this is the correct way of doing this. But please let me know if you've different idea in mind. Thanks for your patience - PR Comment: https://git.openjdk.org/jdk/pull/12283#issuecomment-1489616583
Re: RFR: 8299748: java/util/zip/Deinflate.java failing on s390x [v3]
> DeInflate.java test fails on s390x platform because size for out1 array which > is responsible for storing the compressed data is insufficient. And being > unable to write whole compressed data on array, on s390 whole data can't be > recovered after compression. So this fix increase Array size (for s390). Amit Kumar has updated the pull request incrementally with one additional commit since the last revision: removes out1,out2 parameters - Changes: - all: https://git.openjdk.org/jdk/pull/12283/files - new: https://git.openjdk.org/jdk/pull/12283/files/5a7fa279..bce89892 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=12283=02 - incr: https://webrevs.openjdk.org/?repo=jdk=12283=01-02 Stats: 12 lines in 1 file changed: 0 ins; 3 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/12283.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/12283/head:pull/12283 PR: https://git.openjdk.org/jdk/pull/12283
Re: RFR: 8305092: Improve Thread.sleep(millis, nanos) for sub-millisecond granularity
On Wed, 29 Mar 2023 17:17:29 GMT, Aleksey Shipilev wrote: > Are there specific factors which would make it unreasonable to implement > `sleep` in terms of `parkNanos`? You would need to reissue any unparks received whilst sleeping. Sleeping and parking are distinct thread states so it would be a lot of work in the Java code and in the VM to make such a change. - PR Comment: https://git.openjdk.org/jdk/pull/13225#issuecomment-1489551942
Re: RFR: 8305092: Improve Thread.sleep(millis, nanos) for sub-millisecond granularity
On Wed, 29 Mar 2023 11:28:53 GMT, Aleksey Shipilev wrote:
> Java API has the `Thread.sleep(millis, nanos)` method exposed to users. The
> documentation for that method clearly says the precision and accuracy are
> dependent on the underlying system behavior. However, it always rounds up
> `nanos` to 1ms when doing the actual sleep. This means users cannot do the
> micro-second precision sleeps, even when the underlying platform allows it.
> Sub-millisecond sleeps are useful to build interesting primitives, like the
> rate limiters that run with >1000 RPS.
>
> When faced with this, some users reach for more awkward APIs like
> `java.util.concurrent.locks.LockSupport.parkNanos`. The use of that API for
> sleeps is not in line with its intent, and while it "seems to work", it might
> have interesting interactions with other uses of `LockSupport`. Additionally,
> these "sleeps" are no longer visible to monitoring tools as "normal sleeps",
> e.g. as `Thread.sleep` events. Therefore, it would be prudent to improve
> current `Thread.sleep(millis, nanos)` for sub-millisecond granularity.
>
> Fortunately, the underlying code is almost ready for this, at least on POSIX
> side. I skipped Windows paths, because its timers are still no good. Note
> that on both Linux and MacOS timers oversleep by about 50us. I have a few
> ideas how to improve the accuracy for them, which would be a topic for a
> separate PR.
>
> Additional testing:
> - [x] New regression test
> - [x] New benchmark
> - [x] Linux x86_64 `tier1`
> - [x] Linux AArch64 `tier1`
src/hotspot/share/runtime/javaThread.cpp line 1981:
> 1979: }
> 1980:
> 1981: bool JavaThread::sleep(jlong millis, jint nanos) {
You don't need the overloads at this level - the incoming call should always
have millis and nanos, even if nanos is zero.
src/hotspot/share/utilities/globalDefinitions.hpp line 351:
> 349: millis = max_secs * MILLIUNITS;
> 350: }
> 351: return millis_to_nanos(millis) + nanos;
This can now exceed the max_secs bound though.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/13225#discussion_r1152639527
PR Review Comment: https://git.openjdk.org/jdk/pull/13225#discussion_r1152640056
Re: RFR: 8304585: Method::invoke rewraps InvocationTargetException if a caller-sensitive method throws IAE [v2]
On Thu, 30 Mar 2023 01:15:33 GMT, Mandy Chung wrote: >> A simple fix to `Method::invoke` which wraps IAE with >> `InvocationTargetException` twice if it's thrown by a caller-sensitive >> method which has no adapter. > > Mandy Chung has updated the pull request incrementally with one additional > commit since the last revision: > > the test should fail if Method::invoke doesn't throw Looks fine to me. - Marked as reviewed by jpai (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13233#pullrequestreview-1364192193
Re: RFR: 8304585: Method::invoke rewraps InvocationTargetException if a caller-sensitive method throws IAE [v2]
> A simple fix to `Method::invoke` which wraps IAE with > `InvocationTargetException` twice if it's thrown by a caller-sensitive method > which has no adapter. Mandy Chung has updated the pull request incrementally with one additional commit since the last revision: the test should fail if Method::invoke doesn't throw - Changes: - all: https://git.openjdk.org/jdk/pull/13233/files - new: https://git.openjdk.org/jdk/pull/13233/files/45c7daf0..9af2d663 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=13233=01 - incr: https://webrevs.openjdk.org/?repo=jdk=13233=00-01 Stats: 4 lines in 1 file changed: 3 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/13233.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13233/head:pull/13233 PR: https://git.openjdk.org/jdk/pull/13233
Re: RFR: 8304585: Method::invoke rewraps InvocationTargetException if a caller-sensitive method throws IAE
On Thu, 30 Mar 2023 00:50:11 GMT, Chen Liang wrote:
>> not following what you mean. Can you elaborate?
>
> Though very unlikely, I recommend adding a line of `Assertions.fail("Should
> not reach here");` after the `m.invoke` and before the catch to ensure the
> invocation always throw `InvocationTargetException`.
I see what you mean. They should always throw. I could add assert.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/13233#discussion_r1152630640
Re: RFR: 8304585: Method::invoke rewraps InvocationTargetException if a caller-sensitive method throws IAE
On Thu, 30 Mar 2023 00:40:46 GMT, Mandy Chung wrote:
>> test/jdk/java/lang/reflect/Method/CallerSensitiveMethodInvoke.java line 56:
>>
>>> 54: Method m = Field.class.getDeclaredMethod("get",
>>> Object.class);
>>> 55: m.invoke(f, new Object());
>>> 56: } catch (InvocationTargetException e) {
>>
>> The test should fail if invoke executes without throwing, same for below
>
> not following what you mean. Can you elaborate?
Though very unlikely, I recommend adding a line of `Assertions.fail("Should not
reach here");` after the `m.invoke` and before the catch to ensure the
invocation always throw `InvocationTargetException`.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/13233#discussion_r1152624889
Re: RFR: 8266571: Sequenced Collections [v4]
On Wed, 29 Mar 2023 21:18:04 GMT, Rémi Forax wrote:
>> In the JEP, it says:
>>> Any modifications to the original collection are visible in the view.
>>
>> If we don't have an efficient reversed view, I don't see a point of
>> declaring a collection sequenced; same reason for declaring a
>> sequenced/deque vs. a full-on list with inefficient list random access
>> operations.
>
> The quote is from the javadoc of reversed (see above), it seems the JEP and
> the javadoc disagree :(
>
>> If we don't have an efficient reversed view, I don't see a point of
>> declaring a collection sequenced
>
> Collections in the JDK provides more efficient implementations, this is what
> the code this PR does.
> Providing a default implementation matters more for external libraries.
>
> And that's why i think we do not need the interface SequencedCollection,
> because all these methods can be declared on Collection instead. Adding them
> on Collection has also the added benefit that as a user, all Clojure
> collections or any other implementations not in the JDK also get a good
> enough implementation of the method reversed().
>
> My fear is that if we introduce all these methods on SequencedCollection
> instead of Collection, library implementations will never be updated to use
> SequencedCollection instead of Collection (because implementing
> SequencedCollection requires the library to work only on JDK 21+) while if
> reversed is declared on Collection, library maintainers will have more
> pressure to write an efficient implementation of reversed for their
> implementations (and will be able to do that without requiring the JDK 21).
So your model would be like:
public interface Collection ... {
...
default Collection reversed() {...} // - can dump to array
default void addLast() {...} // or addFirst - no reliable impl
default boolean removeLast() {...} // or removeFirst - no reliable impl
default T getLast() {...} // or getFirst - can dump to array
}
I don't think `reversed()` is the problem here; `addLast` `removeLast` cannot
be implemented from the existing Collection methods without risking contract
violations, so I don't recommend leaving them as default methods. I still think
SequencedCollection should exist for its specification of add/removeLast and
improved performance of getLast.
That said, it is true that SequencedCollection as an interface itself has less
merit as unmodifiable/immutable collections. It will only work as a marker
indicating the contract that an encounter sequence is defined.
For libraries, they can use version-specific class files in META-INF; to
un-sequence a collection implementation should be quite simple, as nuking the
bridge `reversed` returning `SequencedCollection` and the `SequencedCollection`
symbol from interface lists. This can be done with a simple classfile
transformer.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1152623381
Re: RFR: 8305111: Locale.lookupTag has typo in parameter
On Wed, 29 Mar 2023 22:51:12 GMT, Justin Lu wrote: > Small typo fix in Locale.lookupTag > > _tangs_ should be _tags_ Marked as reviewed by naoto (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/13235#pullrequestreview-1364170271
Re: RFR: 8304585: Method::invoke rewraps InvocationTargetException if a caller-sensitive method throws IAE
On Wed, 29 Mar 2023 23:58:59 GMT, Chen Liang wrote:
>> A simple fix to `Method::invoke` which wraps IAE with
>> `InvocationTargetException` twice if it's thrown by a caller-sensitive
>> method which has no adapter.
>
> test/jdk/java/lang/reflect/Method/CallerSensitiveMethodInvoke.java line 56:
>
>> 54: Method m = Field.class.getDeclaredMethod("get",
>> Object.class);
>> 55: m.invoke(f, new Object());
>> 56: } catch (InvocationTargetException e) {
>
> The test should fail if invoke executes without throwing, same for below
not following what you mean. Can you elaborate?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/13233#discussion_r1152621017
Re: RFR: 8304265: Implementation of Foreign Function and Memory API (Third Preview) [v15]
On Wed, 29 Mar 2023 08:55:08 GMT, Per Minborg wrote: >> API changes for the FFM API (third preview) >> >> Specdiff: >> https://cr.openjdk.org/~pminborg/panama/21/v1/specdiff/overview-summary.html >> >> Javadoc: >> https://cr.openjdk.org/~pminborg/panama/21/v1/javadoc/java.base/module-summary.html > > Per Minborg has updated the pull request incrementally with one additional > commit since the last revision: > > Cleanup finality src/java.base/share/classes/java/lang/foreign/MemorySegment.java line 396: > 394: *} > 395: * > 396: * Alternatively, if the size of the foreign segment is known > statically, clients can associate a Suggestion: * Alternatively, if the true size of the zero-length memory segment is known statically, clients can associate a - PR Review Comment: https://git.openjdk.org/jdk/pull/13079#discussion_r1152602321
Re: RFR: 8304585: Method::invoke rewraps InvocationTargetException if a caller-sensitive method throws IAE
On Wed, 29 Mar 2023 21:26:24 GMT, Mandy Chung wrote:
> A simple fix to `Method::invoke` which wraps IAE with
> `InvocationTargetException` twice if it's thrown by a caller-sensitive method
> which has no adapter.
test/jdk/java/lang/reflect/Method/CallerSensitiveMethodInvoke.java line 56:
> 54: Method m = Field.class.getDeclaredMethod("get", Object.class);
> 55: m.invoke(f, new Object());
> 56: } catch (InvocationTargetException e) {
The test should fail if invoke executes without throwing, same for below
-
PR Review Comment: https://git.openjdk.org/jdk/pull/13233#discussion_r1152600270
Re: RFR: 8304265: Implementation of Foreign Function and Memory API (Third Preview) [v15]
On Wed, 29 Mar 2023 22:32:20 GMT, Paul Sandoz wrote: >> I've updated the specs as per how I interpret the comments above. Let me >> know your thoughts on this. > > This is much clearer. Are the names of the struct members guaranteed to be > symbols that can be found up via the linker's default lookup? i.e. a small > set of important global variables. While they might be defined as global variable, I wouldn't read too much into this. Even `errno` is not really a global variable, but some thread local value that the native compiler knows about (e.g. setting the global variable doesn't really do much). Quoting from Linux man page: errno is defined by the ISO C standard to be a modifiable lvalue of type int, and must not be explicitly declared; errno may be a macro. errno is thread-local; setting it in one thread does not affect its value in any other thread. This seems to imply that errno might even be something else. Given how the story goes for errno, I don't think we'd want to restrict the spec to state that these names should be discoverable via the lookup. - PR Review Comment: https://git.openjdk.org/jdk/pull/13079#discussion_r1152598834
Re: RFR: 8304265: Implementation of Foreign Function and Memory API (Third Preview) [v15]
On Wed, 29 Mar 2023 08:55:08 GMT, Per Minborg wrote: >> API changes for the FFM API (third preview) >> >> Specdiff: >> https://cr.openjdk.org/~pminborg/panama/21/v1/specdiff/overview-summary.html >> >> Javadoc: >> https://cr.openjdk.org/~pminborg/panama/21/v1/javadoc/java.base/module-summary.html > > Per Minborg has updated the pull request incrementally with one additional > commit since the last revision: > > Cleanup finality src/java.base/share/classes/java/lang/foreign/MemorySegment.java line 410: > 408: * wrapping a native pointer. For instance, if such pointer points to a > C struct, the client might prefer to resize the > 409: * segment unsafely, to match the size of the struct (so that > out-of-bounds access will be detected by the API). If the > 410: * size is known statically, using an address layout with the correct > target layout might be preferable. Suggestion: * true size of the zero-length memory segment is known statically, using an address layout with the correct target layout might be preferable. - PR Review Comment: https://git.openjdk.org/jdk/pull/13079#discussion_r1152595828
Re: RFR: 8305111: Locale.lookupTag has typo in parameter
On Wed, 29 Mar 2023 22:51:12 GMT, Justin Lu wrote: > Small typo fix in Locale.lookupTag > > _tangs_ should be _tags_ Marked as reviewed by iris (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/13235#pullrequestreview-1364106018
Re: RFR: 8225641: Calendar.roll(int field) does not work correctly for WEEK_OF_YEAR [v5]
On Tue, 28 Mar 2023 21:06:34 GMT, Justin Lu wrote:
>> This PR fixes the bug which occurred when `Calendar.roll(WEEK_OF_YEAR)`
>> rolled into a minimal first week with an invalid `WEEK_OF_YEAR` and
>> `DAY_OF_WEEK` combo.
>>
>> For example, Rolling _Monday, 30 December 2019_ by 1 week produced _Monday,
>> 31 December 2018_, which is incorrect. This is because `WEEK_OF_YEAR` is
>> rolled from 52 to 1, and the original `DAY_OF_WEEK` is 1. However, there is
>> no Monday in week 1 of 2019. This is exposed when a future method calls
>> `Calendar.complete()`, which eventually calculates a `fixedDate` with the
>> invalid `WEEK_OF_YEAR` and `DAY_OF_WEEK` combo.
>>
>> To prevent this, a check is added for rolls into week 1, which determines if
>> the first week is minimal. If it is indeed minimal, then it is checked if
>> `DAY_OF_WEEK` exists in that week, if not, `WEEK_OF_YEAR` must be
>> incremented by one.
>>
>> After the fix, Rolling _Monday, 30 December 2019_ by 1 week produces
>> _Monday, 7 January 2019_
>
> Justin Lu has updated the pull request incrementally with five additional
> commits since the last revision:
>
> - Impl cleanup, add Saturday end day conditional
> - Rename test, clarify test documentation
> - Add type to static declaration of Cal.Builder, since only concerned with
> Gregorian for this test
> - Pass in amount as 1 directly
> - Reuse Calendar.Builder
src/java.base/share/classes/java/util/GregorianCalendar.java line 3014:
> 3012: */
> 3013: private boolean dayInMinWeek (int day, int startDay, int endDay) {
> 3014: endDay = endDay == 0
Why `endDay` can be `0`? If the arguments are `Calendar.XXXDAY`, should they be
between 1 to 7? In other words, why is the above calling site doing
`getFirstDayOfWeek() - 1`?
I'd add some range checks for the input values.
test/jdk/java/util/Calendar/RollFromLastToFirstWeek.java line 52:
> 50: */
> 51: public class RollFromLastToFirstWeek {
> 52: static Calendar.Builder GREGORIAN_BUILDER = new Builder()
Nit: `Calendar.` can be omitted (and can be `private static final`)
-
PR Review Comment: https://git.openjdk.org/jdk/pull/13031#discussion_r1152579044
PR Review Comment: https://git.openjdk.org/jdk/pull/13031#discussion_r1152579338
Re: RFR: 8305111: Locale.lookupTag has typo in parameter
On Wed, 29 Mar 2023 22:51:12 GMT, Justin Lu wrote: > Small typo fix in Locale.lookupTag > > _tangs_ should be _tags_ Marked as reviewed by lancea (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/13235#pullrequestreview-1364100110
Re: RFR: 8304919: Implementation of Virtual Threads [v4]
On Wed, 29 Mar 2023 08:00:36 GMT, Alan Bateman wrote: >> JEP 444 proposes to make virtual threads a permanent feature in Java 21. The >> APIs that were preview APIs in Java 19/20 are changed to permanent and their >> `@since`/equivalent are changed to 21 (as per the guidance in JEP 12). The >> JNI and JVMTI versions are bumped as this is the first change in 21 to need >> the new version number. A lot of tests are updated to drop `@enablePreview` >> and --enable-preview. >> >> There is one API change from Java 19/20, the preview API >> Thread.Builder.allowSetThreadLocals(boolean) is dropped. This requires an >> update to the JVMTI GetThreadInfo implementation to read the TCCL >> consistently. >> >> In addition, there are a small number of implementation changes to sync up >> from the loom fibers branch: >> >> - A number of stack frames are `@Hidden` to reduce noise in the stack >> traces. This exposed a few issues with the stack walker code. More >> specifically, the cases where end of a continuation falls precisely at the >> end of the batch, or where the remaining frames are hidden, weren't handled >> correctly. >> - The code to emit the JFR jdk.ThreadSleepEvent is refactored so it's in >> Thread rather than in two classes. >> - A few robustness improvements for OOME and SOE. There is more to do here, >> for future PRs. >> - New system property to print a stack trace when a virtual thread sets its >> own value of a TL. >> - ThreadPerTaskExecutor is changed to use FutureTask. >> >> Testing: tier1-6. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Fix ThreadSleepEvent again The stack walker change looks good. I also reviewed other changes which look fine to me. - Marked as reviewed by mchung (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13203#pullrequestreview-1364091585
Is there such a thing as a customizable SAX Contenthandler which works as a XPath observer of sorts?
SAX Content handlers do parse data in one go with very little to no overhead whatsoever, but for each XML file with a differently layout or if you need something different out of the same XML file, you will need to code a corresponding content handler. There is also the problem that not all XML-ish data is nicely written with the data types in a serial order. I think there should be something like what I need even if partially (if I am making sense ;-)). Which libraries implement such a thing? Or how would you suggest such a library should be implemented? lbrtchx
RFR: 8305111: Locale.lookupTag has typo in parameter
Small typo fix in Locale.lookupTag _tangs_ should be _tags_ - Commit messages: - Fix typo Changes: https://git.openjdk.org/jdk/pull/13235/files Webrev: https://webrevs.openjdk.org/?repo=jdk=13235=00 Issue: https://bugs.openjdk.org/browse/JDK-8305111 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/13235.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13235/head:pull/13235 PR: https://git.openjdk.org/jdk/pull/13235
Re: RFR: 8304871: Use default visibility for static library builds [v2]
On Mon, 27 Mar 2023 09:40:22 GMT, Severin Gehwolf wrote: >> Please review this change for symbol visibility of static library artefacts. >> This fixes an issue when >> OpenJDK is being used as a base for generating native images preventing the >> symbols from being >> exported and looked up from the executable. Note that symbol visibity is now >> controlled by a >> linker version script downstream. This changes nothing for the regularly >> shipped dynamic libraries. >> >> Thoughts? > > Severin Gehwolf has updated the pull request incrementally with one > additional commit since the last revision: > > Explicitly set the JNI_EXPORT define for static libs Marked as reviewed by dholmes (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/13177#pullrequestreview-1364077182
Re: RFR: 8304265: Implementation of Foreign Function and Memory API (Third Preview) [v15]
On Wed, 22 Mar 2023 14:07:14 GMT, Per Minborg wrote: >> Back to @PaulSandoz question - "how does the caller know what the structure >> contains?". The caller typically doesn't care too much about what the >> returned struct is. But it might care about which "values" can be captured. >> That said, the set of such interesting values, is not too surprising. As >> demonstrated in the example in the `Linker.capturedCallState` method, once >> the client knows the name (and "errno" is likely to be 90% case), everything >> else follows from there - as the layout can be used to obtain var handles >> for the required values. >> >> But, perhaps, now that I write this, I realize that what @PaulSandoz might >> _really_ be asking is: how do I know that e.g. the returned struct will not >> contain e.g. nested structs, sequences, or other non-sense. So we might >> specify (in a normative way) that the returned layout is a struct layout, >> whose member layouts are one or more value layouts (possibly with some added >> padding layouts). The names of the value layouts reflect the names of the >> values that can be captured. >> >> And _then_ we show an example of the layout we return for Windows/x64 (as >> that's more interesting). > > I've updated the specs as per how I interpret the comments above. Let me know > your thoughts on this. This is much clearer. Are the names of the struct members guaranteed to be symbols that can be found up via the linker's default lookup? i.e. a small set of important global variables. - PR Review Comment: https://git.openjdk.org/jdk/pull/13079#discussion_r1152550849
Re: RFR: 8304919: Implementation of Virtual Threads [v4]
On Wed, 29 Mar 2023 08:00:36 GMT, Alan Bateman wrote: >> JEP 444 proposes to make virtual threads a permanent feature in Java 21. The >> APIs that were preview APIs in Java 19/20 are changed to permanent and their >> `@since`/equivalent are changed to 21 (as per the guidance in JEP 12). The >> JNI and JVMTI versions are bumped as this is the first change in 21 to need >> the new version number. A lot of tests are updated to drop `@enablePreview` >> and --enable-preview. >> >> There is one API change from Java 19/20, the preview API >> Thread.Builder.allowSetThreadLocals(boolean) is dropped. This requires an >> update to the JVMTI GetThreadInfo implementation to read the TCCL >> consistently. >> >> In addition, there are a small number of implementation changes to sync up >> from the loom fibers branch: >> >> - A number of stack frames are `@Hidden` to reduce noise in the stack >> traces. This exposed a few issues with the stack walker code. More >> specifically, the cases where end of a continuation falls precisely at the >> end of the batch, or where the remaining frames are hidden, weren't handled >> correctly. >> - The code to emit the JFR jdk.ThreadSleepEvent is refactored so it's in >> Thread rather than in two classes. >> - A few robustness improvements for OOME and SOE. There is more to do here, >> for future PRs. >> - New system property to print a stack trace when a virtual thread sets its >> own value of a TL. >> - ThreadPerTaskExecutor is changed to use FutureTask. >> >> Testing: tier1-6. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Fix ThreadSleepEvent again Looks good. test/jdk/java/lang/Thread/virtual/TraceVirtualThreadLocals.java line 65: > 63: > 64: /** > 65: * Runs a task in a virutal thread, returning a String with any > output printed Suggestion: * Runs a task in a virtual thread, returning a String with any output printed - Marked as reviewed by psandoz (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13203#pullrequestreview-1364039280 PR Review Comment: https://git.openjdk.org/jdk/pull/13203#discussion_r1152529524
Re: RFR: 8304585: Method::invoke rewraps InvocationTargetException if a caller-sensitive method throws IAE
On Wed, 29 Mar 2023 21:26:24 GMT, Mandy Chung wrote: > A simple fix to `Method::invoke` which wraps IAE with > `InvocationTargetException` twice if it's thrown by a caller-sensitive method > which has no adapter. Looks fine. - Marked as reviewed by darcy (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13233#pullrequestreview-1364018900
Re: RFR: 8266571: Sequenced Collections [v4]
On Wed, 29 Mar 2023 20:05:10 GMT, Rémi Forax wrote: >> @forax but this would not be a view: changes in the underlying collection >> won't be reflected > > Yes, > The spec says :"Changes to the underlying collection might or might not be > visible in this reversed view, depending upon the implementation." so i > believe the default implementation i proposed is a valid implementation In the JEP, it says: > Any modifications to the original collection are visible in the view. If we don't have an efficient reversed view, I don't see a point of declaring a collection sequenced; same reason for declaring a sequenced/deque vs. a full-on list with inefficient list random access operations. - PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1152474419
Re: RFR: 8266571: Sequenced Collections [v4]
On Wed, 29 Mar 2023 20:54:15 GMT, Chen Liang wrote: >> Yes, >> The spec says :"Changes to the underlying collection might or might not be >> visible in this reversed view, depending upon the implementation." so i >> believe the default implementation i proposed is a valid implementation > > In the JEP, it says: >> Any modifications to the original collection are visible in the view. > > If we don't have an efficient reversed view, I don't see a point of declaring > a collection sequenced; same reason for declaring a sequenced/deque vs. a > full-on list with inefficient list random access operations. The quote is from the javadoc of reversed (see above), it seems the JEP and the javadoc disagree :( > If we don't have an efficient reversed view, I don't see a point of declaring > a collection sequenced Collections in the JDK provides more efficient implementations, this is what the code this PR does. Providing a default implementation matters more for external libraries. And that's why i think we do not need the interface SequencedCollection, because all these methods can be declared on Collection instead. Adding them on Collection has also the added benefit that as a user, all Clojure collections or any other implementations not in the JDK also get a good enough implementation of the method reversed(). My fear is that if we introduce all these methods on SequencedCollection instead of Collection, library implementations will never be updated to use SequencedCollection instead of Collection (because implementing SequencedCollection requires the library to work only on JDK 21+) while if reversed is declared on Collection, library maintainers will have more pressure to write an efficient implementation of reversed for their implementations (and will be able to do that without requiring the JDK 21). - PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1152494872
Re: RFR: 8266571: Sequenced Collections [v4]
On Tue, 28 Mar 2023 02:37:22 GMT, Stuart Marks wrote: >> PR for Sequenced Collections implementation. > > Stuart Marks has updated the pull request incrementally with one additional > commit since the last revision: > > Simplify handling of cached keySet, values, and entrySet views. In the JEP: > A sequenced collection supports common operations at either end, and it > supports processing the elements from first to last and from last to first > (i.e., forward and reverse). > The reverse-ordered view enables all the different sequenced types to process > elements in both directions, using all the usual iteration mechanisms: > Enhanced for loops, explicit iterator() loops, forEach(), stream(), > parallelStream(), and toArray(). This implies that for the reversed view, collection operations are not only supported, but can potentially be optimized. Our design should anticipate implementations of `SequencedCollection` to provide specific reversed implementations, like what we are already doing with `addAll` in ArrayList and ArrayDeque. If a collection cannot have efficient reverse-sequenced operations, it shouldn't be retrofitted into `SequencedCollection`, just like how we don't fit Deques into Lists. Hence, I recommend: 1. Declare `reversed()` and one of the (First/Last) operation sets (add|get|remove) `abstract`, and delegae the other set to default call the operation on the reversed instead. - Since we tend to work with the end of collections, I'd declare the `Last` methods to be abstract and delegate the `First` default implementations to `this.reversed().xxxLast()` 2. I don't think leaving `addLast()` implemented by default is a good idea, for modifiable implementations cannot discover that they missed the `addLast()` at compile time and can only discover when the implementation is actually used. 3. In the default `reversed()` implementation of `List` and `Deque`, add API notes to indicate to implementations opportunities to optimize the implementation, especially batch operations like `addAll` when the base implementation offers such an optimization. - PR Comment: https://git.openjdk.org/jdk/pull/7387#issuecomment-1489338414
Re: RFR: 8304006: jlink should create the jimage file in the native endian for the target platform [v16]
On Tue, 28 Mar 2023 11:02:46 GMT, Alan Bateman wrote: >> Jaikiran Pai has updated the pull request incrementally with one additional >> commit since the last revision: >> >> use test.jdk system property in test > > test/jdk/tools/jlink/plugins/CDSPluginTest.java line 97: > >> 95:// separate --module-path will force the JLink task to read >> the ModuleTarget from >> 96:// the java.base module-info.class to identify the target >> platform for the image >> 97:// being generated. > > I'm a bit uncomfortable with change the test for the CDS plugin as part of > this change but can live with it. Early on during this PR, we decided and implemented to read the `ModuleTarget` only if the java.base module's file path doesn't match that of the current platform's file path. If we remove that check and instead always read the ModuleTarget of java.base, in the JLinkTask, we won't need any changes to this test, since the JLinkTask will rightly find the correct target platform from the module-info.class. Furthermore, rest of the cross platform image generation would work fine too. Do you think we should remove that file path check? I am guessing the reason why the file path checks were suggested was to reduce any chances of regression with these current changes? - PR Review Comment: https://git.openjdk.org/jdk/pull/11943#discussion_r1151311196
RFR: 8304585: Method::invoke rewraps InvocationTargetException if a caller-sensitive method throws IAE
A simple fix to `Method::invoke` which wraps IAE with `InvocationTargetException` twice if it's thrown by a caller-sensitive method which has no adapter. - Commit messages: - 8304585: Method::invoke rewraps InvocationTargetException if a caller-sensitive method throws IAE Changes: https://git.openjdk.org/jdk/pull/13233/files Webrev: https://webrevs.openjdk.org/?repo=jdk=13233=00 Issue: https://bugs.openjdk.org/browse/JDK-8304585 Stats: 87 lines in 2 files changed: 81 ins; 4 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/13233.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13233/head:pull/13233 PR: https://git.openjdk.org/jdk/pull/13233
Re: RFR: 8304919: Implementation of Virtual Threads [v4]
On Wed, 29 Mar 2023 08:00:36 GMT, Alan Bateman wrote: >> JEP 444 proposes to make virtual threads a permanent feature in Java 21. The >> APIs that were preview APIs in Java 19/20 are changed to permanent and their >> `@since`/equivalent are changed to 21 (as per the guidance in JEP 12). The >> JNI and JVMTI versions are bumped as this is the first change in 21 to need >> the new version number. A lot of tests are updated to drop `@enablePreview` >> and --enable-preview. >> >> There is one API change from Java 19/20, the preview API >> Thread.Builder.allowSetThreadLocals(boolean) is dropped. This requires an >> update to the JVMTI GetThreadInfo implementation to read the TCCL >> consistently. >> >> In addition, there are a small number of implementation changes to sync up >> from the loom fibers branch: >> >> - A number of stack frames are `@Hidden` to reduce noise in the stack >> traces. This exposed a few issues with the stack walker code. More >> specifically, the cases where end of a continuation falls precisely at the >> end of the batch, or where the remaining frames are hidden, weren't handled >> correctly. >> - The code to emit the JFR jdk.ThreadSleepEvent is refactored so it's in >> Thread rather than in two classes. >> - A few robustness improvements for OOME and SOE. There is more to do here, >> for future PRs. >> - New system property to print a stack trace when a virtual thread sets its >> own value of a TL. >> - ThreadPerTaskExecutor is changed to use FutureTask. >> >> Testing: tier1-6. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Fix ThreadSleepEvent again src/java.base/share/classes/java/lang/Thread.java line 1546: > 1544: // bind thread to container > 1545: if (this.container != null) > 1546: throw new IllegalThreadStateException(); This check is not replicated in `VirtualThread::start`, i think the CAS protects against that. Maybe assert instead in the virtual thread implementation, thereby the comment in `setThreadContainer` can be changed to something like "`this.container` checked/asserted to be != null before call to Virtual/Thread::start(ThreadContainer)" ? - PR Review Comment: https://git.openjdk.org/jdk/pull/13203#discussion_r1152514040
Re: RFR: 8304919: Implementation of Virtual Threads [v4]
On Wed, 29 Mar 2023 08:00:36 GMT, Alan Bateman wrote: >> JEP 444 proposes to make virtual threads a permanent feature in Java 21. The >> APIs that were preview APIs in Java 19/20 are changed to permanent and their >> `@since`/equivalent are changed to 21 (as per the guidance in JEP 12). The >> JNI and JVMTI versions are bumped as this is the first change in 21 to need >> the new version number. A lot of tests are updated to drop `@enablePreview` >> and --enable-preview. >> >> There is one API change from Java 19/20, the preview API >> Thread.Builder.allowSetThreadLocals(boolean) is dropped. This requires an >> update to the JVMTI GetThreadInfo implementation to read the TCCL >> consistently. >> >> In addition, there are a small number of implementation changes to sync up >> from the loom fibers branch: >> >> - A number of stack frames are `@Hidden` to reduce noise in the stack >> traces. This exposed a few issues with the stack walker code. More >> specifically, the cases where end of a continuation falls precisely at the >> end of the batch, or where the remaining frames are hidden, weren't handled >> correctly. >> - The code to emit the JFR jdk.ThreadSleepEvent is refactored so it's in >> Thread rather than in two classes. >> - A few robustness improvements for OOME and SOE. There is more to do here, >> for future PRs. >> - New system property to print a stack trace when a virtual thread sets its >> own value of a TL. >> - ThreadPerTaskExecutor is changed to use FutureTask. >> >> Testing: tier1-6. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Fix ThreadSleepEvent again Test changes looks good. - Marked as reviewed by lmesnik (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13203#pullrequestreview-1363976888
Re: RFR: 8303530: Add system property for custom JAXP configuration file [v5]
> Add a system property, jdk.xml.config.file, to return the path to a custom > JAXP configuration file. The current configuration file, jaxp.properties, > that the JDK supports will become the default configuration file. > > CSR: https://bugs.openjdk.org/browse/JDK-8303531 > > Tests: XML SQE and JCK tests passed. Joe Wang has updated the pull request incrementally with one additional commit since the last revision: change prefix from jdk to java - Changes: - all: https://git.openjdk.org/jdk/pull/12985/files - new: https://git.openjdk.org/jdk/pull/12985/files/b9dff937..aa7db3ae Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=12985=04 - incr: https://webrevs.openjdk.org/?repo=jdk=12985=03-04 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/12985.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/12985/head:pull/12985 PR: https://git.openjdk.org/jdk/pull/12985
Re: RFR: 8303530: Add system property for custom JAXP configuration file [v4]
> Add a system property, jdk.xml.config.file, to return the path to a custom > JAXP configuration file. The current configuration file, jaxp.properties, > that the JDK supports will become the default configuration file. > > CSR: https://bugs.openjdk.org/browse/JDK-8303531 > > Tests: XML SQE and JCK tests passed. Joe Wang has updated the pull request incrementally with one additional commit since the last revision: change prefix from jdk to java; rm zip file that accidentally checked in - Changes: - all: https://git.openjdk.org/jdk/pull/12985/files - new: https://git.openjdk.org/jdk/pull/12985/files/43407bdc..b9dff937 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=12985=03 - incr: https://webrevs.openjdk.org/?repo=jdk=12985=02-03 Stats: 0 lines in 1 file changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/12985.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/12985/head:pull/12985 PR: https://git.openjdk.org/jdk/pull/12985
Re: RFR: 8304919: Implementation of Virtual Threads [v4]
On Wed, 29 Mar 2023 08:00:36 GMT, Alan Bateman wrote: >> JEP 444 proposes to make virtual threads a permanent feature in Java 21. The >> APIs that were preview APIs in Java 19/20 are changed to permanent and their >> `@since`/equivalent are changed to 21 (as per the guidance in JEP 12). The >> JNI and JVMTI versions are bumped as this is the first change in 21 to need >> the new version number. A lot of tests are updated to drop `@enablePreview` >> and --enable-preview. >> >> There is one API change from Java 19/20, the preview API >> Thread.Builder.allowSetThreadLocals(boolean) is dropped. This requires an >> update to the JVMTI GetThreadInfo implementation to read the TCCL >> consistently. >> >> In addition, there are a small number of implementation changes to sync up >> from the loom fibers branch: >> >> - A number of stack frames are `@Hidden` to reduce noise in the stack >> traces. This exposed a few issues with the stack walker code. More >> specifically, the cases where end of a continuation falls precisely at the >> end of the batch, or where the remaining frames are hidden, weren't handled >> correctly. >> - The code to emit the JFR jdk.ThreadSleepEvent is refactored so it's in >> Thread rather than in two classes. >> - A few robustness improvements for OOME and SOE. There is more to do here, >> for future PRs. >> - New system property to print a stack trace when a virtual thread sets its >> own value of a TL. >> - ThreadPerTaskExecutor is changed to use FutureTask. >> >> Testing: tier1-6. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Fix ThreadSleepEvent again Serviceability changes look good. - Marked as reviewed by cjplummer (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13203#pullrequestreview-1363917524
Re: RFR: 8266571: Sequenced Collections [v4]
On Wed, 29 Mar 2023 19:54:48 GMT, Tagir F. Valeev wrote: >> In the same spirit, `reversed()` should also have a default implementation >> equivalent to >> >> >> Collections.unmodifiableSequenceCollection(Arrays.asList(this.toArray())).reversed() > > @forax but this would not be a view: changes in the underlying collection > won't be reflected Yes, The spec says :"Changes to the underlying collection might or might not be visible in this reversed view, depending upon the implementation." so i believe the default implementation i proposed is a valid implementation - PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1152426988
Re: RFR: 8305092: Improve Thread.sleep(millis, nanos) for sub-millisecond granularity
On Wed, 29 Mar 2023 18:29:18 GMT, Aleksey Shipilev wrote: > Yes, let me fix that. `TimeUnit.toNanos` handles it well itself, it seems. This code is refactored in PR 13203 so we'll have to merge at some point. - PR Review Comment: https://git.openjdk.org/jdk/pull/13225#discussion_r1152419710
Re: RFR: 8266571: Sequenced Collections [v4]
On Wed, 29 Mar 2023 19:20:16 GMT, Rémi Forax wrote:
>> src/java.base/share/classes/java/util/SequencedCollection.java line 107:
>>
>>> 105: */
>>> 106: default void addFirst(E e) {
>>> 107: throw new UnsupportedOperationException();
>>
>> Can this be defaulted to `this.reversed().addLast()` instead? If this throws
>> uoe, the reversed should throw uoe as well; and the new default can simplify
>> implementations by much as well.
>
> In the same spirit, `reversed()` should also have a default implementation
> equivalent to
>
>
> Collections.unmodifiableSequenceCollection(Arrays.asList(this.toArray())).reversed()
@forax but this would not be a view: changes in the underlying collection won't
be reflected
-
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1152416692
Re: RFR: 8302819: Remove JAR Index [v2]
On Tue, 28 Mar 2023 20:06:03 GMT, Mandy Chung wrote:
>> Eirik Bjorsnos 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 11 additional
>> commits since the last revision:
>>
>> - Remove blank line 93 as per suggestion by Mandy
>> - Merge branch 'master' into remove-jar-index
>> - Remove JarIndex default constructor
>> - Make JarIndex.INDEX_NAME package private
>> - Remove outdated reference to URLClassLoader using JarIndex
>> - Make JarIndex package private
>> - The JarIndex.merge method ended up being used only by the
>> JarIndexMergeTest test. Removing this test, the JarIndex.merge methods and a
>> number of other JarIndex methods not used by the 'jar' tool
>> - Revert the export of sun.security.action to jdk.jartool. JarIndex can use
>> System.property instead since the 'jar' tool does not run with a
>> SecurityManager
>> - Revert noisy whitespace changes
>> - Revert noisy whitespace changes
>> - ... and 1 more: https://git.openjdk.org/jdk/compare/599939df...2b5d8a4a
>
> src/java.base/share/classes/jdk/internal/loader/URLClassPath.java line 936:
>
>> 934: @Override
>> 935: URL[] getClassPath() throws IOException {
>> 936:
>
> Nit: extra line 936 can be removed.
Thank to @mlchung, line 936 is now history!
-
PR Review Comment: https://git.openjdk.org/jdk/pull/13158#discussion_r1152412960
Re: RFR: 8302819: Remove JAR Index [v2]
> This PR removes the JAR index feature from the runtime: > > - `URLClassPath` is updated to remove the `enableJarIndex` system property > and any code which would be called when this property was `true` > - The `JarIndex` implementation class is moved into `jdk.jartool` module. > - The `InvalidJarIndexError` exception class is removed because it falls out > of use > - The test `test/jdk/sun/misc/JarIndex/metaInfFileNames/Basic.java` is > removed because it depends on the JarIndex feature being present > - The test `test/jdk/sun/misc/JarIndex/JarIndexMergeForClassLoaderTest.java` > is removed because it depends on the JarIndex feature being present > - The test `test/jdk/sun/misc/JarIndex/JarIndexMergeTest.java` is removed > because it end up being the only caller of the JarIndex.merge feature > - All `JarIndex` methods/constructors which are not used by the `jar -i` > implementation are removed. > - `JarIndex` is given package-private access. > > Outstanding code work: > > - Create tests for `JarFile` and `JarInputStream` accepting dusty INDEX jars. > > Outstanding work: > > - CSR for the removal > - Release notes for the removal > - Coordination of the update of the Jar File Specification Eirik Bjorsnos 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 11 additional commits since the last revision: - Remove blank line 93 as per suggestion by Mandy - Merge branch 'master' into remove-jar-index - Remove JarIndex default constructor - Make JarIndex.INDEX_NAME package private - Remove outdated reference to URLClassLoader using JarIndex - Make JarIndex package private - The JarIndex.merge method ended up being used only by the JarIndexMergeTest test. Removing this test, the JarIndex.merge methods and a number of other JarIndex methods not used by the 'jar' tool - Revert the export of sun.security.action to jdk.jartool. JarIndex can use System.property instead since the 'jar' tool does not run with a SecurityManager - Revert noisy whitespace changes - Revert noisy whitespace changes - ... and 1 more: https://git.openjdk.org/jdk/compare/599939df...2b5d8a4a - Changes: - all: https://git.openjdk.org/jdk/pull/13158/files - new: https://git.openjdk.org/jdk/pull/13158/files/fee0da27..2b5d8a4a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=13158=01 - incr: https://webrevs.openjdk.org/?repo=jdk=13158=00-01 Stats: 12002 lines in 289 files changed: 4769 ins; 5962 del; 1271 mod Patch: https://git.openjdk.org/jdk/pull/13158.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13158/head:pull/13158 PR: https://git.openjdk.org/jdk/pull/13158
Re: RFR: 8304919: Implementation of Virtual Threads [v4]
On Wed, 29 Mar 2023 18:30:01 GMT, Chris Plummer wrote: >> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix ThreadSleepEvent again > > test/hotspot/jtreg/serviceability/jvmti/thread/GetFrameCount/framecnt01/framecnt01.java > line 82: > >> 80: >> 81: // this is too fragile, implementation can change at any time. >> 82: checkFrames(vThread1, false, 14); > > Is this due to the `@hidden` being added to `Continuation.enter()` and > `enter0()`? If so, since both methods are now hidden, why are there not 2 > fewer frames? Was there also an additional frame added somewhere? No, it's that a lambda expression is replaced in the implementation. - PR Review Comment: https://git.openjdk.org/jdk/pull/13203#discussion_r1152402303
Re: RFR: 8304919: Implementation of Virtual Threads [v2]
On Wed, 29 Mar 2023 18:47:03 GMT, Chris Plummer wrote: >> tid0 is the thread ID of a platform therad. tid1 is the threadID of a >> virtual thread. The only change here is allow this test run with the main >> wrapper plugin >> ([CODETOOLS-7903373](https://bugs.openjdk.org/browse/CODETOOLS-7903373)), it >> would otherwise have to be excluded for those runs. > > I don't see any problemlist changes. Was this test failing when using the > wrapper because of the lack of problemlisting? I added this test recently via JDK-8303242. It failed when we sync'ed up the loom repo as the test configuration there runs the tests with the jtreg main wrapper. It was trivial to fix and this avoid needing to exclude it via ProblemList-Virtual.txt. Once jtreg is promoted and there is config added to run the tests with the virtual ThreadFactory then this test will be able to run in this mode. - PR Review Comment: https://git.openjdk.org/jdk/pull/13203#discussion_r1152393905
Re: RFR: 8304014: Convert test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java to junit [v3]
On Wed, 29 Mar 2023 19:03:08 GMT, Daniel Fuchs wrote: >> Scratch that. It seems my IDE was just not being very cooperative in >> suggesting or finding JUnit 5 API. When I wrote the imports myself it seems >> to have improved its behaviour. >> >> Do you think it looks better now, @dfuch ? > > Yes - this a recent effort. I believe you will need at least jtreg 7, and > JUnit 5.8.2 It is worth noting that I did a new local build of jtreg and that may have updated the junit API jar. I would find that weird because the file name or version of the JAR file was not updated `jtreg/build/deps/junit/junit-platform-console-standalone-1.9.2.jar`. But maybe the content of the file changed? - PR Review Comment: https://git.openjdk.org/jdk/pull/12563#discussion_r1152384929
Re: RFR: 8266571: Sequenced Collections [v4]
On Wed, 29 Mar 2023 19:06:20 GMT, Chen Liang wrote:
>> Stuart Marks has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Simplify handling of cached keySet, values, and entrySet views.
>
> src/java.base/share/classes/java/util/SequencedCollection.java line 107:
>
>> 105: */
>> 106: default void addFirst(E e) {
>> 107: throw new UnsupportedOperationException();
>
> Can this be defaulted to `this.reversed().addLast()` instead? If this throws
> uoe, the reversed should throw uoe as well; and the new default can simplify
> implementations by much as well.
In the same spirit, `reversed()` should also have a default implementation
equivalent to
Collections.unmodifiableSequenceCollection(Arrays.asList(this.toArray())).reversed()
-
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1152384938
Re: RFR: 8266571: Sequenced Collections [v4]
On Tue, 28 Mar 2023 02:37:22 GMT, Stuart Marks wrote:
>> PR for Sequenced Collections implementation.
>
> Stuart Marks has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Simplify handling of cached keySet, values, and entrySet views.
src/java.base/share/classes/java/util/SequencedCollection.java line 107:
> 105: */
> 106: default void addFirst(E e) {
> 107: throw new UnsupportedOperationException();
Can this be defaulted to `this.reversed().addLast()` instead? If this throws
uoe, the reversed should throw uoe as well; and the new default can simplify
implementations by much as well.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1152372181
Re: RFR: 8304014: Convert test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java to junit [v3]
On Wed, 29 Mar 2023 18:50:27 GMT, Eirik Bjorsnos wrote: >> Seems my dev environment has `jtreg` < 7. Maybe that's a problem? > > Seems junit 5 support is a recent effort? > > https://mail.openjdk.org/pipermail/jdk-dev/2022-August/006869.html Scratch that. It seems my IDE was just not being very cooperative in suggesting or finding JUnit 5 API. When I wrote the imports myself it seems to have improved its behaviour. Do you think it looks better now, @dfuch ? - PR Review Comment: https://git.openjdk.org/jdk/pull/12563#discussion_r1152369091
Re: RFR: 8304014: Convert test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java to junit [v3]
On Wed, 29 Mar 2023 19:02:56 GMT, Eirik Bjorsnos wrote: >> Seems junit 5 support is a recent effort? >> >> https://mail.openjdk.org/pipermail/jdk-dev/2022-August/006869.html > > Scratch that. It seems my IDE was just not being very cooperative in > suggesting or finding JUnit 5 API. When I wrote the imports myself it seems > to have improved its behaviour. > > Do you think it looks better now, @dfuch ? Yes - this a recent effort. I believe you will need at least jtreg 7, and JUnit 5.8.2 - PR Review Comment: https://git.openjdk.org/jdk/pull/12563#discussion_r1152369274
Re: RFR: 8304014: Convert test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java to junit [v4]
> CorruptedZipFiles could benefit from some spring cleaning and a conversion to > testNG: > > - The actual tests are moved into their own `@Test` methods, given more > meaningful names and a Javadoc comment explaining the constraint being > verified > - The setup code is moved to a `@Before` method, slightly modernized and > rewritten to take advantage of `assertEquals` > - `checkZipExceptionImpl` is updated to take advantage of `expectThrows` > - A bunch of constants copied over from `ZipFile` can be deleted since > JDK-6225935 has long been fixed Eirik Bjorsnos has updated the pull request incrementally with one additional commit since the last revision: Update annotations to junit 5 - Changes: - all: https://git.openjdk.org/jdk/pull/12563/files - new: https://git.openjdk.org/jdk/pull/12563/files/892e166a..01c25e84 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=12563=03 - incr: https://webrevs.openjdk.org/?repo=jdk=12563=02-03 Stats: 7 lines in 1 file changed: 0 ins; 0 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/12563.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/12563/head:pull/12563 PR: https://git.openjdk.org/jdk/pull/12563
Re: RFR: 8304919: Implementation of Virtual Threads [v2]
On Wed, 29 Mar 2023 07:27:50 GMT, Alan Bateman wrote:
>> test/jdk/com/sun/management/ThreadMXBean/VirtualThreads.java line 143:
>>
>>> 141: long[] tids = new long[] { tid0, tid1 };
>>> 142: long[] cpuTimes = bean.getThreadCpuTime(tids);
>>> 143: if (Thread.currentThread().isVirtual()) {
>>
>> How it worked before?
>
> tid0 is the thread ID of a platform therad. tid1 is the threadID of a virtual
> thread. The only change here is allow this test run with the main wrapper
> plugin
> ([CODETOOLS-7903373](https://bugs.openjdk.org/browse/CODETOOLS-7903373)), it
> would otherwise have to be excluded for those runs.
I don't see any problemlist changes. Was this test failing when using the
wrapper because of the lack of problemlisting?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/13203#discussion_r1152353646
Re: RFR: 8304919: Implementation of Virtual Threads [v4]
On Wed, 29 Mar 2023 08:00:36 GMT, Alan Bateman wrote: >> JEP 444 proposes to make virtual threads a permanent feature in Java 21. The >> APIs that were preview APIs in Java 19/20 are changed to permanent and their >> `@since`/equivalent are changed to 21 (as per the guidance in JEP 12). The >> JNI and JVMTI versions are bumped as this is the first change in 21 to need >> the new version number. A lot of tests are updated to drop `@enablePreview` >> and --enable-preview. >> >> There is one API change from Java 19/20, the preview API >> Thread.Builder.allowSetThreadLocals(boolean) is dropped. This requires an >> update to the JVMTI GetThreadInfo implementation to read the TCCL >> consistently. >> >> In addition, there are a small number of implementation changes to sync up >> from the loom fibers branch: >> >> - A number of stack frames are `@Hidden` to reduce noise in the stack >> traces. This exposed a few issues with the stack walker code. More >> specifically, the cases where end of a continuation falls precisely at the >> end of the batch, or where the remaining frames are hidden, weren't handled >> correctly. >> - The code to emit the JFR jdk.ThreadSleepEvent is refactored so it's in >> Thread rather than in two classes. >> - A few robustness improvements for OOME and SOE. There is more to do here, >> for future PRs. >> - New system property to print a stack trace when a virtual thread sets its >> own value of a TL. >> - ThreadPerTaskExecutor is changed to use FutureTask. >> >> Testing: tier1-6. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Fix ThreadSleepEvent again test/hotspot/jtreg/serviceability/jvmti/thread/GetFrameCount/framecnt01/framecnt01.java line 82: > 80: > 81: // this is too fragile, implementation can change at any time. > 82: checkFrames(vThread1, false, 14); Is this due to the `@hidden` being added to `Continuation.enter()` and `enter0()`? If so, since both methods are now hidden, why are there not 2 fewer frames? Was there also an additional frame added somewhere? - PR Review Comment: https://git.openjdk.org/jdk/pull/13203#discussion_r1152337485
Re: RFR: 8304014: Convert test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java to junit [v3]
On Wed, 29 Mar 2023 18:38:00 GMT, Eirik Bjorsnos wrote: >> Noob question: Do you know where I find the jars for setting up junit5 in my >> IDE? jtreg's `junit-platform-console-standalone-1.9.2` does not seem to >> include these annotations. > > Seems my dev environment has `jtreg` < 7. Maybe that's a problem? Seems junit 5 support is a recent effort? https://mail.openjdk.org/pipermail/jdk-dev/2022-August/006869.html - PR Review Comment: https://git.openjdk.org/jdk/pull/12563#discussion_r1152356869
Re: RFR: 8304014: Convert test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java to junit [v3]
On Wed, 29 Mar 2023 18:35:04 GMT, Eirik Bjorsnos wrote: >> test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java line 36: >> >>> 34: import org.junit.BeforeClass; >>> 35: import org.junit.Test; >>> 36: >> >> I believe you should be using corresponding annotations from >> `org.junit.jupiter.api` instead. >> From >> https://junit.org/junit5/docs/current/user-guide/index.html#migrating-from-junit4 >>> `@Before` and `@After` no longer exist; use `@BeforeEach` and `@AfterEach` >>> instead. >>> `@BeforeClass` and `@AfterClass` no longer exist; use `@BeforeAll` and >>> `@AfterAll` instead. >> >> Similarly the `@Test` annotation should be imported from >> `org.junit.jupiter.api` > > Noob question: Do you know where I find the jars for setting up junit5 in my > IDE? jtreg's `junit-platform-console-standalone-1.9.2` does not seem to > include these annotations. Seems my dev environment has `jtreg` < 7. Maybe that's a problem? - PR Review Comment: https://git.openjdk.org/jdk/pull/12563#discussion_r1152344743
Re: RFR: 8304014: Convert test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java to junit [v3]
On Wed, 29 Mar 2023 18:25:34 GMT, Daniel Fuchs wrote: >> Eirik Bjorsnos has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Junit version > > test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java line 36: > >> 34: import org.junit.BeforeClass; >> 35: import org.junit.Test; >> 36: > > I believe you should be using corresponding annotations from > `org.junit.jupiter.api` instead. > From > https://junit.org/junit5/docs/current/user-guide/index.html#migrating-from-junit4 >> `@Before` and `@After` no longer exist; use `@BeforeEach` and `@AfterEach` >> instead. >> `@BeforeClass` and `@AfterClass` no longer exist; use `@BeforeAll` and >> `@AfterAll` instead. > > Similarly the `@Test` annotation should be imported from > `org.junit.jupiter.api` Noob question: Do you know where I find the jars for setting up junit5 in my IDE? jtreg's `junit-platform-console-standalone-1.9.2` does not seem to include these annotations. - PR Review Comment: https://git.openjdk.org/jdk/pull/12563#discussion_r1152342103
Integrated: 8304976: Optimize DateTimeFormatterBuilder.ZoneTextPrinterParser.getTree()
On Fri, 17 Feb 2023 09:50:16 GMT, Sergey Tsypanov wrote:
> 1) When `DateTimeFormatter` is reused we don't need to copy
> `availableZoneIds` and allocate `nonRegionIds` as PrefixTree can be taken
> from cache. In the related benchmark allocation of `HashSet` takes ~93% of
> all time, so avoiding it should bring some improvement for cases when we
> reuse `DateTimeFormatter` to parse a string into `ZoneDateTime`
> 
>
> 2) `DateTimeFormatter` is mostly used with one locale, so `cachedTree` and
> `cachedTreeCI` can have predefined size.
>
>
> @State(Scope.Thread)
> @BenchmarkMode(Mode.AverageTime)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class DateTimeFormatterBenchmark {
>
> private static final DateTimeFormatter df = new
> DateTimeFormatterBuilder().appendPattern(":MM:dd:HH:mm:v").toFormatter();
> private static final String TEXT = "2015:03:10:12:13:ECT";
>
> @Setup
> public void setUp() {
> ZonedDateTime.parse(TEXT, df);
> }
>
> @Benchmark
> public ZonedDateTime parse() {
> return ZonedDateTime.parse(TEXT, df);
> }
> }
This pull request has now been integrated.
Changeset: 438c969b
Author:Sergey Tsypanov
Committer: Naoto Sato
URL:
https://git.openjdk.org/jdk/commit/438c969b7b07eeef0158b089e5a168849e04bf56
Stats: 62 lines in 2 files changed: 57 ins; 1 del; 4 mod
8304976: Optimize DateTimeFormatterBuilder.ZoneTextPrinterParser.getTree()
Reviewed-by: naoto
-
PR: https://git.openjdk.org/jdk/pull/12612
Re: RFR: 8305092: Improve Thread.sleep(millis, nanos) for sub-millisecond granularity
On Wed, 29 Mar 2023 18:10:30 GMT, Alan Bateman wrote:
>> Java API has the `Thread.sleep(millis, nanos)` method exposed to users. The
>> documentation for that method clearly says the precision and accuracy are
>> dependent on the underlying system behavior. However, it always rounds up
>> `nanos` to 1ms when doing the actual sleep. This means users cannot do the
>> micro-second precision sleeps, even when the underlying platform allows it.
>> Sub-millisecond sleeps are useful to build interesting primitives, like the
>> rate limiters that run with >1000 RPS.
>>
>> When faced with this, some users reach for more awkward APIs like
>> `java.util.concurrent.locks.LockSupport.parkNanos`. The use of that API for
>> sleeps is not in line with its intent, and while it "seems to work", it
>> might have interesting interactions with other uses of `LockSupport`.
>> Additionally, these "sleeps" are no longer visible to monitoring tools as
>> "normal sleeps", e.g. as `Thread.sleep` events. Therefore, it would be
>> prudent to improve current `Thread.sleep(millis, nanos)` for sub-millisecond
>> granularity.
>>
>> Fortunately, the underlying code is almost ready for this, at least on POSIX
>> side. I skipped Windows paths, because its timers are still no good. Note
>> that on both Linux and MacOS timers oversleep by about 50us. I have a few
>> ideas how to improve the accuracy for them, which would be a topic for a
>> separate PR.
>>
>> Additional testing:
>> - [x] New regression test
>> - [x] New benchmark
>> - [x] Linux x86_64 `tier1`
>> - [x] Linux AArch64 `tier1`
>
> src/java.base/share/classes/java/lang/Thread.java line 509:
>
>> 507: ThreadSleepEvent event = new ThreadSleepEvent();
>> 508: try {
>> 509: event.time = MILLISECONDS.toNanos(millis) + nanos;
>
> This can overflow when millis is at or close to Long.MAX_VALUE.
Yes, let me fix that. `TimeUnit.toNanos` handles it well itself, it seems.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/13225#discussion_r1152336826
Re: RFR: 8304014: Convert test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java to junit [v3]
On Wed, 29 Mar 2023 18:15:42 GMT, Eirik Bjorsnos wrote: >> CorruptedZipFiles could benefit from some spring cleaning and a conversion >> to testNG: >> >> - The actual tests are moved into their own `@Test` methods, given more >> meaningful names and a Javadoc comment explaining the constraint being >> verified >> - The setup code is moved to a `@Before` method, slightly modernized and >> rewritten to take advantage of `assertEquals` >> - `checkZipExceptionImpl` is updated to take advantage of `expectThrows` >> - A bunch of constants copied over from `ZipFile` can be deleted since >> JDK-6225935 has long been fixed > > Eirik Bjorsnos has updated the pull request incrementally with one additional > commit since the last revision: > > Junit version Hi Eirik, thanks for following up on my suggestion :-) test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java line 36: > 34: import org.junit.BeforeClass; > 35: import org.junit.Test; > 36: I believe you should be using corresponding annotations from `org.junit.jupiter.api` instead. >From >https://junit.org/junit5/docs/current/user-guide/index.html#migrating-from-junit4 > `@Before` and `@After` no longer exist; use `@BeforeEach` and `@AfterEach` > instead. > `@BeforeClass` and `@AfterClass` no longer exist; use `@BeforeAll` and > `@AfterAll` instead. Similarly the `@Test` annotation should be imported from `org.junit.jupiter.api` - Changes requested by dfuchs (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/12563#pullrequestreview-1363720139 PR Review Comment: https://git.openjdk.org/jdk/pull/12563#discussion_r1152333259
Re: RFR: 8305092: Improve Thread.sleep(millis, nanos) for sub-millisecond granularity
On Wed, 29 Mar 2023 11:28:53 GMT, Aleksey Shipilev wrote: > Java API has the `Thread.sleep(millis, nanos)` method exposed to users. The > documentation for that method clearly says the precision and accuracy are > dependent on the underlying system behavior. However, it always rounds up > `nanos` to 1ms when doing the actual sleep. This means users cannot do the > micro-second precision sleeps, even when the underlying platform allows it. > Sub-millisecond sleeps are useful to build interesting primitives, like the > rate limiters that run with >1000 RPS. > > When faced with this, some users reach for more awkward APIs like > `java.util.concurrent.locks.LockSupport.parkNanos`. The use of that API for > sleeps is not in line with its intent, and while it "seems to work", it might > have interesting interactions with other uses of `LockSupport`. Additionally, > these "sleeps" are no longer visible to monitoring tools as "normal sleeps", > e.g. as `Thread.sleep` events. Therefore, it would be prudent to improve > current `Thread.sleep(millis, nanos)` for sub-millisecond granularity. > > Fortunately, the underlying code is almost ready for this, at least on POSIX > side. I skipped Windows paths, because its timers are still no good. Note > that on both Linux and MacOS timers oversleep by about 50us. I have a few > ideas how to improve the accuracy for them, which would be a topic for a > separate PR. > > Additional testing: > - [x] New regression test > - [x] New benchmark > - [x] Linux x86_64 `tier1` > - [x] Linux AArch64 `tier1` Should we manifest this change in JavaDoc somehow? Just to let people know that starting from the next Java release they can use sub-millisecond intervals in their code. - PR Comment: https://git.openjdk.org/jdk/pull/13225#issuecomment-1489084409
RFR: 8132995: Matcher$ImmutableMatchResult should be optimized to reduce space usage
When appropriate and useful, copies only the relevant portion of the `CharSequence` to the match result. - Commit messages: - 8132995: Matcher should be optimized to reduce space usage Changes: https://git.openjdk.org/jdk/pull/13231/files Webrev: https://webrevs.openjdk.org/?repo=jdk=13231=00 Issue: https://bugs.openjdk.org/browse/JDK-8132995 Stats: 111 lines in 2 files changed: 97 ins; 2 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/13231.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13231/head:pull/13231 PR: https://git.openjdk.org/jdk/pull/13231
Re: RFR: 8304014: Convert test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java to junit [v2]
On Wed, 29 Mar 2023 15:24:41 GMT, Lance Andersen wrote: > Let's go with reviewing this version, Thank you for the update Eirik Good, I pushed the junit version to the PR. Also updated the JBS and PR titles. Big thanks to @dfuch for suggesting, your lower-level comments are also welcome! - PR Comment: https://git.openjdk.org/jdk/pull/12563#issuecomment-1489072396
Re: RFR: 8304014: Convert test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java to junit [v3]
> CorruptedZipFiles could benefit from some spring cleaning and a conversion to > testNG: > > - The actual tests are moved into their own `@Test` methods, given more > meaningful names and a Javadoc comment explaining the constraint being > verified > - The setup code is moved to a `@Before` method, slightly modernized and > rewritten to take advantage of `assertEquals` > - `checkZipExceptionImpl` is updated to take advantage of `expectThrows` > - A bunch of constants copied over from `ZipFile` can be deleted since > JDK-6225935 has long been fixed Eirik Bjorsnos has updated the pull request incrementally with one additional commit since the last revision: Junit version - Changes: - all: https://git.openjdk.org/jdk/pull/12563/files - new: https://git.openjdk.org/jdk/pull/12563/files/e46fc5b3..892e166a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=12563=02 - incr: https://webrevs.openjdk.org/?repo=jdk=12563=01-02 Stats: 24 lines in 1 file changed: 3 ins; 0 del; 21 mod Patch: https://git.openjdk.org/jdk/pull/12563.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/12563/head:pull/12563 PR: https://git.openjdk.org/jdk/pull/12563
Re: RFR: 8305092: Improve Thread.sleep(millis, nanos) for sub-millisecond granularity
On Wed, 29 Mar 2023 11:28:53 GMT, Aleksey Shipilev wrote:
> Java API has the `Thread.sleep(millis, nanos)` method exposed to users. The
> documentation for that method clearly says the precision and accuracy are
> dependent on the underlying system behavior. However, it always rounds up
> `nanos` to 1ms when doing the actual sleep. This means users cannot do the
> micro-second precision sleeps, even when the underlying platform allows it.
> Sub-millisecond sleeps are useful to build interesting primitives, like the
> rate limiters that run with >1000 RPS.
>
> When faced with this, some users reach for more awkward APIs like
> `java.util.concurrent.locks.LockSupport.parkNanos`. The use of that API for
> sleeps is not in line with its intent, and while it "seems to work", it might
> have interesting interactions with other uses of `LockSupport`. Additionally,
> these "sleeps" are no longer visible to monitoring tools as "normal sleeps",
> e.g. as `Thread.sleep` events. Therefore, it would be prudent to improve
> current `Thread.sleep(millis, nanos)` for sub-millisecond granularity.
>
> Fortunately, the underlying code is almost ready for this, at least on POSIX
> side. I skipped Windows paths, because its timers are still no good. Note
> that on both Linux and MacOS timers oversleep by about 50us. I have a few
> ideas how to improve the accuracy for them, which would be a topic for a
> separate PR.
>
> Additional testing:
> - [x] New regression test
> - [x] New benchmark
> - [x] Linux x86_64 `tier1`
> - [x] Linux AArch64 `tier1`
src/java.base/share/classes/java/lang/Thread.java line 509:
> 507: ThreadSleepEvent event = new ThreadSleepEvent();
> 508: try {
> 509: event.time = MILLISECONDS.toNanos(millis) + nanos;
This can overflow when millis is at or close to Long.MAX_VALUE.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/13225#discussion_r1152318322
Re: RFR: 8298619: java/io/File/GetXSpace.java is failing [v4]
> Modify the `Space` instances used for size comparison to be created with > total number of bytes derived from the Windows `diskFree` utility instead of > Cygwin’s `df`. Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 8305157: Clean up - Changes: - all: https://git.openjdk.org/jdk/pull/12397/files - new: https://git.openjdk.org/jdk/pull/12397/files/114857c1..9ae317e7 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=12397=03 - incr: https://webrevs.openjdk.org/?repo=jdk=12397=02-03 Stats: 14 lines in 2 files changed: 7 ins; 6 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/12397.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/12397/head:pull/12397 PR: https://git.openjdk.org/jdk/pull/12397
Integrated: 8303392: Runtime.exec and ProcessBuilder.start should use System logger
On Fri, 3 Mar 2023 19:26:52 GMT, Roger Riggs wrote: > Runtime.exec and ProcessBuilder.start methods create a new operating system > process with the program and arguments. Many applications configure a logging > subsystem to monitor application events. Logging a process start message with > the program, arguments, and stack trace can identify the caller and purpose. > Logging the process start event is complementary to the process start event > generated for JFR (Java Flight Recorder). This pull request has now been integrated. Changeset: d063b896 Author:Roger Riggs URL: https://git.openjdk.org/jdk/commit/d063b8964fbdd6ca1d9527dabb40fed59bbc8ad7 Stats: 272 lines in 6 files changed: 272 ins; 0 del; 0 mod 8303392: Runtime.exec and ProcessBuilder.start should use System logger Reviewed-by: stuefe, alanb, mullan - PR: https://git.openjdk.org/jdk/pull/12862
Re: RFR: 8305092: Improve Thread.sleep(millis, nanos) for sub-millisecond granularity
On Wed, 29 Mar 2023 11:28:53 GMT, Aleksey Shipilev wrote: > Java API has the `Thread.sleep(millis, nanos)` method exposed to users. The > documentation for that method clearly says the precision and accuracy are > dependent on the underlying system behavior. However, it always rounds up > `nanos` to 1ms when doing the actual sleep. This means users cannot do the > micro-second precision sleeps, even when the underlying platform allows it. > Sub-millisecond sleeps are useful to build interesting primitives, like the > rate limiters that run with >1000 RPS. > > When faced with this, some users reach for more awkward APIs like > `java.util.concurrent.locks.LockSupport.parkNanos`. The use of that API for > sleeps is not in line with its intent, and while it "seems to work", it might > have interesting interactions with other uses of `LockSupport`. Additionally, > these "sleeps" are no longer visible to monitoring tools as "normal sleeps", > e.g. as `Thread.sleep` events. Therefore, it would be prudent to improve > current `Thread.sleep(millis, nanos)` for sub-millisecond granularity. > > Fortunately, the underlying code is almost ready for this, at least on POSIX > side. I skipped Windows paths, because its timers are still no good. Note > that on both Linux and MacOS timers oversleep by about 50us. I have a few > ideas how to improve the accuracy for them, which would be a topic for a > separate PR. > > Additional testing: > - [x] New regression test > - [x] New benchmark > - [x] Linux x86_64 `tier1` > - [x] Linux AArch64 `tier1` Right, I was assuming that such an implementation would be conformant to the existing specification of the `sleep` method, thus it would have to loop by consuming the unpark (if any), checking for interruption, parking for the remaining time, and reinstating the unpark at the end (if any was consumed). The main risk that I could see of such an approach is that if the sleep was interrupted, then the loop would necessarily unpark itself again (as interruption also interrupts park but this is indistinguishable from an explicit unpark), which might cause a spurious unpark down the line. But it should be pretty consistent with how it is used in interruptible locks AFAIK. I was mostly curious about other factors like JVMTI and thread status though. - PR Comment: https://git.openjdk.org/jdk/pull/13225#issuecomment-1489010885
Re: RFR: 8305092: Improve Thread.sleep(millis, nanos) for sub-millisecond granularity
On Wed, 29 Mar 2023 16:48:41 GMT, David M. Lloyd wrote:
> Are there specific factors which would make it unreasonable to implement
> `sleep` in terms of `parkNanos`?
After reading the Javadoc for `LockSupport`, I don't believe implementing
`Thread.sleep` with `LockSupport.parkNanos` is a proper thing to do.
`LockSupport` implicitly deals with permits, acting like a single-permit
semaphore. So the behavior for `parkNanos` when there is a prior
`LockSupport.unpark` (which adds the permit) is to consume the permit and
return immediately.
In other words:
% cat LockSupportIsNotSleep.java
import java.util.concurrent.locks.LockSupport;
import java.util.concurrent.TimeUnit;
public class LockSupportIsNotSleep {
public static void main(String... args) {
LockSupport.unpark(Thread.currentThread());
long time1 = System.nanoTime();
LockSupport.parkNanos(TimeUnit.SECONDS.toNanos(1000));
long time2 = System.nanoTime();
System.out.println("Slept for " + (time2 - time1) + " ns");
}
}
% java LockSupportIsNotSleep.java
Slept for 6583 ns
-
PR Comment: https://git.openjdk.org/jdk/pull/13225#issuecomment-1489000794
Re: RFR: 8305092: Improve Thread.sleep(millis, nanos) for sub-millisecond granularity
On Wed, 29 Mar 2023 11:28:53 GMT, Aleksey Shipilev wrote: > Java API has the `Thread.sleep(millis, nanos)` method exposed to users. The > documentation for that method clearly says the precision and accuracy are > dependent on the underlying system behavior. However, it always rounds up > `nanos` to 1ms when doing the actual sleep. This means users cannot do the > micro-second precision sleeps, even when the underlying platform allows it. > Sub-millisecond sleeps are useful to build interesting primitives, like the > rate limiters that run with >1000 RPS. > > When faced with this, some users reach for more awkward APIs like > `java.util.concurrent.locks.LockSupport.parkNanos`. The use of that API for > sleeps is not in line with its intent, and while it "seems to work", it might > have interesting interactions with other uses of `LockSupport`. Additionally, > these "sleeps" are no longer visible to monitoring tools as "normal sleeps", > e.g. as `Thread.sleep` events. Therefore, it would be prudent to improve > current `Thread.sleep(millis, nanos)` for sub-millisecond granularity. > > Fortunately, the underlying code is almost ready for this, at least on POSIX > side. I skipped Windows paths, because its timers are still no good. Note > that on both Linux and MacOS timers oversleep by about 50us. I have a few > ideas how to improve the accuracy for them, which would be a topic for a > separate PR. > > Additional testing: > - [x] New regression test > - [x] New benchmark > - [x] Linux x86_64 `tier1` > - [x] Linux AArch64 `tier1` Motivational improvements on new benchmark below: m6g.9xlarge (Linux, Xeon, x86_64): Benchmark (sleep) Mode Cnt Score Error Units # = no regressions for Thread.sleep(millis) # Baseline ThreadSleep.millis 0 avgt5 463.606 ± 8.256 ns/op ThreadSleep.millis 1 avgt5 459.434 ± 0.453 ns/op ThreadSleep.millis10 avgt5 462.828 ± 1.283 ns/op ThreadSleep.millis 100 avgt5 459.495 ± 3.244 ns/op ThreadSleep.millis 1000 avgt5 457.962 ± 5.769 ns/op ThreadSleep.millis 1 avgt5 457.960 ± 1.195 ns/op ThreadSleep.millis10 avgt5 459.197 ± 3.054 ns/op ThreadSleep.millis 100 avgt5 1058342.873 ± 304.169 ns/op ThreadSleep.millis 1000 avgt510059945.148 ± 1642.411 ns/op ThreadSleep.millis 1 avgt5 100062181.340 ± 1863.186 ns/op ThreadSleep.millis10 avgt5 170015.600 ± 11379.631 ns/op # Patched ThreadSleep.millis 0 avgt5 461.295 ± 6.700 ns/op ThreadSleep.millis 1 avgt5 463.383 ± 3.065 ns/op ThreadSleep.millis10 avgt5 468.109 ± 1.607 ns/op ThreadSleep.millis 100 avgt5 467.316 ± 0.270 ns/op ThreadSleep.millis 1000 avgt5 461.999 ± 2.341 ns/op ThreadSleep.millis 1 avgt5 465.075 ± 0.324 ns/op ThreadSleep.millis10 avgt5 462.588 ± 5.321 ns/op ThreadSleep.millis 100 avgt5 1058285.440 ± 409.376 ns/op ThreadSleep.millis 1000 avgt510060437.490 ± 177.754 ns/op ThreadSleep.millis 1 avgt5 100060665.980 ± 1167.514 ns/op ThreadSleep.millis10 avgt5 171917.300 ± 17438.906 ns/op # = Significant improvements for Thread.sleep(millis, nanos) # Baseline ThreadSleep.millisNanos 0 avgt5 462.831 ± 0.251 ns/op ThreadSleep.millisNanos 1 avgt5 1058708.668 ± 960.683 ns/op ThreadSleep.millisNanos 10 avgt5 1058364.414 ± 197.700 ns/op ThreadSleep.millisNanos 100 avgt5 1058451.889 ± 561.979 ns/op ThreadSleep.millisNanos1000 avgt5 1058372.010 ±96.867 ns/op ThreadSleep.millisNanos 1 avgt5 1058354.100 ± 297.148 ns/op ThreadSleep.millisNanos 10 avgt5 1058485.092 ± 1093.340 ns/op ThreadSleep.millisNanos 100 avgt5 1059004.036 ± 1899.950 ns/op ThreadSleep.millisNanos1000 avgt510059733.302 ± 1725.908 ns/op ThreadSleep.millisNanos 1 avgt5 100063327.840 ± 6244.805 ns/op ThreadSleep.millisNanos 10 avgt5 174542.300 ± 17898.173 ns/op # Patched ThreadSleep.millisNanos 0 avgt5 467.968 ± 9.202 ns/op ThreadSleep.millisNanos 1 avgt5 579.448 ± 1.363 ns/op ThreadSleep.millisNanos 10 avgt5 577.656 ± 0.278 ns/op ThreadSleep.millisNanos 100 avgt5 58092.815 ± 2410.401 ns/op ThreadSleep.millisNanos1000 avgt5 58592.249 ± 189.232 ns/op
Re: RFR: 8305092: Improve Thread.sleep(millis, nanos) for sub-millisecond granularity
On Wed, 29 Mar 2023 11:28:53 GMT, Aleksey Shipilev wrote: > Java API has the `Thread.sleep(millis, nanos)` method exposed to users. The > documentation for that method clearly says the precision and accuracy are > dependent on the underlying system behavior. However, it always rounds up > `nanos` to 1ms when doing the actual sleep. This means users cannot do the > micro-second precision sleeps, even when the underlying platform allows it. > Sub-millisecond sleeps are useful to build interesting primitives, like the > rate limiters that run with >1000 RPS. > > When faced with this, some users reach for more awkward APIs like > `java.util.concurrent.locks.LockSupport.parkNanos`. The use of that API for > sleeps is not in line with its intent, and while it "seems to work", it might > have interesting interactions with other uses of `LockSupport`. Additionally, > these "sleeps" are no longer visible to monitoring tools as "normal sleeps", > e.g. as `Thread.sleep` events. Therefore, it would be prudent to improve > current `Thread.sleep(millis, nanos)` for sub-millisecond granularity. > > Fortunately, the underlying code is almost ready for this, at least on POSIX > side. I skipped Windows paths, because its timers are still no good. Note > that on both Linux and MacOS timers oversleep by about 50us. I have a few > ideas how to improve the accuracy for them, which would be a topic for a > separate PR. > > Motivational improvements on new benchmark below: > > m6g.9xlarge (Linux, Xeon, x86_64): > > > Benchmark (sleep) Mode Cnt Score Error > Units > > # = no regressions for Thread.sleep(millis) > > # Baseline > ThreadSleep.millis 0 avgt5 463.606 ± 8.256 ns/op > ThreadSleep.millis 1 avgt5 459.434 ± 0.453 ns/op > ThreadSleep.millis10 avgt5 462.828 ± 1.283 ns/op > ThreadSleep.millis 100 avgt5 459.495 ± 3.244 ns/op > ThreadSleep.millis 1000 avgt5 457.962 ± 5.769 ns/op > ThreadSleep.millis 1 avgt5 457.960 ± 1.195 ns/op > ThreadSleep.millis10 avgt5 459.197 ± 3.054 ns/op > ThreadSleep.millis 100 avgt5 1058342.873 ± 304.169 ns/op > ThreadSleep.millis 1000 avgt510059945.148 ± 1642.411 ns/op > ThreadSleep.millis 1 avgt5 100062181.340 ± 1863.186 ns/op > ThreadSleep.millis10 avgt5 170015.600 ± 11379.631 ns/op > > # Patched > ThreadSleep.millis 0 avgt5 461.295 ± 6.700 ns/op > ThreadSleep.millis 1 avgt5 463.383 ± 3.065 ns/op > ThreadSleep.millis10 avgt5 468.109 ± 1.607 ns/op > ThreadSleep.millis 100 avgt5 467.316 ± 0.270 ns/op > ThreadSleep.millis 1000 avgt5 461.999 ± 2.341 ns/op > ThreadSleep.millis 1 avgt5 465.075 ± 0.324 ns/op > ThreadSleep.millis10 avgt5 462.588 ± 5.321 ns/op > ThreadSleep.millis 100 avgt5 1058285.440 ± 409.376 ns/op > ThreadSleep.millis 1000 avgt510060437.490 ± 177.754 ns/op > ThreadSleep.millis 1 avgt5 100060665.980 ± 1167.514 ns/op > ThreadSleep.millis10 avgt5 171917.300 ± 17438.906 ns/op > > # = Significant improvements for Thread.sleep(millis, nanos) > > # Baseline > ThreadSleep.millisNanos 0 avgt5 462.831 ± 0.251 > ns/op > ThreadSleep.millisNanos 1 avgt5 1058708.668 ± 960.683 > ns/op > ThreadSleep.millisNanos 10 avgt5 1058364.414 ± 197.700 > ns/op > ThreadSleep.millisNanos 100 avgt5 1058451.889 ± 561.979 > ns/op > ThreadSleep.millisNanos1000 avgt5 1058372.010 ±96.867 > ns/op > ThreadSleep.millisNanos 1 avgt5 1058354.100 ± 297.148 > ns/op > ThreadSleep.millisNanos 10 avgt5 1058485.092 ± 1093.340 > ns/op > ThreadSleep.millisNanos 100 avgt5 1059004.036 ± 1899.950 > ns/op > ThreadSleep.millisNanos1000 avgt510059733.302 ± 1725.908 > ns/op > ThreadSleep.millisNanos 1 avgt5 100063327.840 ± 6244.805 > ns/op > ThreadSleep.millisNanos 10 avgt5 174542.300 ± 17898.173 > ns/op > > # Patched > ThreadSleep.millisNanos 0 avgt5 467.968 ± 9.202 > ns/op > ThreadSleep.millisNanos 1 avgt5 579.448 ± 1.363 > ns/op > ThreadSleep.millisNanos 10 avgt5 577.656 ± 0.278 > ns/op > ThreadSleep.millisNanos 100 avgt5 58092.815 ± 2410.401 > ns/op > ThreadSleep.millisNanos1000 avgt5 58592.249 ± 189.232 > ns/op >
Re: RFR: 8305092: Improve Thread.sleep(millis, nanos) for sub-millisecond granularity
On Wed, 29 Mar 2023 11:28:53 GMT, Aleksey Shipilev wrote: > Java API has the `Thread.sleep(millis, nanos)` method exposed to users. The > documentation for that method clearly says the precision and accuracy are > dependent on the underlying system behavior. However, it always rounds up > `nanos` to 1ms when doing the actual sleep. This means users cannot do the > micro-second precision sleeps, even when the underlying platform allows it. > Sub-millisecond sleeps are useful to build interesting primitives, like the > rate limiters that run with >1000 RPS. > > When faced with this, some users reach for more awkward APIs like > `java.util.concurrent.locks.LockSupport.parkNanos`. The use of that API for > sleeps is not in line with its intent, and while it "seems to work", it might > have interesting interactions with other uses of `LockSupport`. Additionally, > these "sleeps" are no longer visible to monitoring tools as "normal sleeps", > e.g. as `Thread.sleep` events. Therefore, it would be prudent to improve > current `Thread.sleep(millis, nanos)` for sub-millisecond granularity. > > Fortunately, the underlying code is almost ready for this, at least on POSIX > side. I skipped Windows paths, because its timers are still no good. Note > that on both Linux and MacOS timers oversleep by about 50us. I have a few > ideas how to improve the accuracy for them, which would be a topic for a > separate PR. > > Motivational improvements on new benchmark below: > > m6g.9xlarge (Linux, Xeon, x86_64): > > > Benchmark (sleep) Mode Cnt Score Error > Units > > # = no regressions for Thread.sleep(millis) > > # Baseline > ThreadSleep.millis 0 avgt5 463.606 ± 8.256 ns/op > ThreadSleep.millis 1 avgt5 459.434 ± 0.453 ns/op > ThreadSleep.millis10 avgt5 462.828 ± 1.283 ns/op > ThreadSleep.millis 100 avgt5 459.495 ± 3.244 ns/op > ThreadSleep.millis 1000 avgt5 457.962 ± 5.769 ns/op > ThreadSleep.millis 1 avgt5 457.960 ± 1.195 ns/op > ThreadSleep.millis10 avgt5 459.197 ± 3.054 ns/op > ThreadSleep.millis 100 avgt5 1058342.873 ± 304.169 ns/op > ThreadSleep.millis 1000 avgt510059945.148 ± 1642.411 ns/op > ThreadSleep.millis 1 avgt5 100062181.340 ± 1863.186 ns/op > ThreadSleep.millis10 avgt5 170015.600 ± 11379.631 ns/op > > # Patched > ThreadSleep.millis 0 avgt5 461.295 ± 6.700 ns/op > ThreadSleep.millis 1 avgt5 463.383 ± 3.065 ns/op > ThreadSleep.millis10 avgt5 468.109 ± 1.607 ns/op > ThreadSleep.millis 100 avgt5 467.316 ± 0.270 ns/op > ThreadSleep.millis 1000 avgt5 461.999 ± 2.341 ns/op > ThreadSleep.millis 1 avgt5 465.075 ± 0.324 ns/op > ThreadSleep.millis10 avgt5 462.588 ± 5.321 ns/op > ThreadSleep.millis 100 avgt5 1058285.440 ± 409.376 ns/op > ThreadSleep.millis 1000 avgt510060437.490 ± 177.754 ns/op > ThreadSleep.millis 1 avgt5 100060665.980 ± 1167.514 ns/op > ThreadSleep.millis10 avgt5 171917.300 ± 17438.906 ns/op > > # = Significant improvements for Thread.sleep(millis, nanos) > > # Baseline > ThreadSleep.millisNanos 0 avgt5 462.831 ± 0.251 > ns/op > ThreadSleep.millisNanos 1 avgt5 1058708.668 ± 960.683 > ns/op > ThreadSleep.millisNanos 10 avgt5 1058364.414 ± 197.700 > ns/op > ThreadSleep.millisNanos 100 avgt5 1058451.889 ± 561.979 > ns/op > ThreadSleep.millisNanos1000 avgt5 1058372.010 ±96.867 > ns/op > ThreadSleep.millisNanos 1 avgt5 1058354.100 ± 297.148 > ns/op > ThreadSleep.millisNanos 10 avgt5 1058485.092 ± 1093.340 > ns/op > ThreadSleep.millisNanos 100 avgt5 1059004.036 ± 1899.950 > ns/op > ThreadSleep.millisNanos1000 avgt510059733.302 ± 1725.908 > ns/op > ThreadSleep.millisNanos 1 avgt5 100063327.840 ± 6244.805 > ns/op > ThreadSleep.millisNanos 10 avgt5 174542.300 ± 17898.173 > ns/op > > # Patched > ThreadSleep.millisNanos 0 avgt5 467.968 ± 9.202 > ns/op > ThreadSleep.millisNanos 1 avgt5 579.448 ± 1.363 > ns/op > ThreadSleep.millisNanos 10 avgt5 577.656 ± 0.278 > ns/op > ThreadSleep.millisNanos 100 avgt5 58092.815 ± 2410.401 > ns/op > ThreadSleep.millisNanos1000 avgt5 58592.249 ± 189.232 > ns/op >
RFR: 8305092: Improve Thread.sleep(millis, nanos) for sub-millisecond granularity
Java API has the `Thread.sleep(millis, nanos)` method exposed to users. The documentation for that method clearly says the precision and accuracy are dependent on the underlying system behavior. However, it always rounds up `nanos` to 1ms when doing the actual sleep. This means users cannot do the micro-second precision sleeps, even when the underlying platform allows it. Sub-millisecond sleeps are useful to build interesting primitives, like the rate limiters that run with >1000 RPS. When faced with this, some users reach for more awkward APIs like `java.util.concurrent.locks.LockSupport.parkNanos`. The use of that API for sleeps is not in line with its intent, and while it "seems to work", it might have interesting interactions with other uses of `LockSupport`. Additionally, these "sleeps" are no longer visible to monitoring tools as "normal sleeps", e.g. as `Thread.sleep` events. Therefore, it would be prudent to improve current `Thread.sleep(millis, nanos)` for sub-millisecond granularity. Fortunately, the underlying code is almost ready for this, at least on POSIX side. I skipped Windows paths, because its timers are still no good. Note that on both Linux and MacOS timers oversleep by about 50us. I have a few ideas how to improve the accuracy for them, which would be a topic for a separate PR. Motivational improvements on new benchmark below: m6g.9xlarge (Linux, Xeon, x86_64): Benchmark (sleep) Mode Cnt Score Error Units # = no regressions for Thread.sleep(millis) # Baseline ThreadSleep.millis 0 avgt5 463.606 ± 8.256 ns/op ThreadSleep.millis 1 avgt5 459.434 ± 0.453 ns/op ThreadSleep.millis10 avgt5 462.828 ± 1.283 ns/op ThreadSleep.millis 100 avgt5 459.495 ± 3.244 ns/op ThreadSleep.millis 1000 avgt5 457.962 ± 5.769 ns/op ThreadSleep.millis 1 avgt5 457.960 ± 1.195 ns/op ThreadSleep.millis10 avgt5 459.197 ± 3.054 ns/op ThreadSleep.millis 100 avgt5 1058342.873 ± 304.169 ns/op ThreadSleep.millis 1000 avgt510059945.148 ± 1642.411 ns/op ThreadSleep.millis 1 avgt5 100062181.340 ± 1863.186 ns/op ThreadSleep.millis10 avgt5 170015.600 ± 11379.631 ns/op # Patched ThreadSleep.millis 0 avgt5 461.295 ± 6.700 ns/op ThreadSleep.millis 1 avgt5 463.383 ± 3.065 ns/op ThreadSleep.millis10 avgt5 468.109 ± 1.607 ns/op ThreadSleep.millis 100 avgt5 467.316 ± 0.270 ns/op ThreadSleep.millis 1000 avgt5 461.999 ± 2.341 ns/op ThreadSleep.millis 1 avgt5 465.075 ± 0.324 ns/op ThreadSleep.millis10 avgt5 462.588 ± 5.321 ns/op ThreadSleep.millis 100 avgt5 1058285.440 ± 409.376 ns/op ThreadSleep.millis 1000 avgt510060437.490 ± 177.754 ns/op ThreadSleep.millis 1 avgt5 100060665.980 ± 1167.514 ns/op ThreadSleep.millis10 avgt5 171917.300 ± 17438.906 ns/op # = Significant improvements for Thread.sleep(millis, nanos) # Baseline ThreadSleep.millisNanos 0 avgt5 462.831 ± 0.251 ns/op ThreadSleep.millisNanos 1 avgt5 1058708.668 ± 960.683 ns/op ThreadSleep.millisNanos 10 avgt5 1058364.414 ± 197.700 ns/op ThreadSleep.millisNanos 100 avgt5 1058451.889 ± 561.979 ns/op ThreadSleep.millisNanos1000 avgt5 1058372.010 ±96.867 ns/op ThreadSleep.millisNanos 1 avgt5 1058354.100 ± 297.148 ns/op ThreadSleep.millisNanos 10 avgt5 1058485.092 ± 1093.340 ns/op ThreadSleep.millisNanos 100 avgt5 1059004.036 ± 1899.950 ns/op ThreadSleep.millisNanos1000 avgt510059733.302 ± 1725.908 ns/op ThreadSleep.millisNanos 1 avgt5 100063327.840 ± 6244.805 ns/op ThreadSleep.millisNanos 10 avgt5 174542.300 ± 17898.173 ns/op # Patched ThreadSleep.millisNanos 0 avgt5 467.968 ± 9.202 ns/op ThreadSleep.millisNanos 1 avgt5 579.448 ± 1.363 ns/op ThreadSleep.millisNanos 10 avgt5 577.656 ± 0.278 ns/op ThreadSleep.millisNanos 100 avgt5 58092.815 ± 2410.401 ns/op ThreadSleep.millisNanos1000 avgt5 58592.249 ± 189.232 ns/op ThreadSleep.millisNanos 1 avgt5 67668.688 ± 591.365 ns/op ThreadSleep.millisNanos 10 avgt5 158143.164 ± 2281.540 ns/op ThreadSleep.millisNanos 100 avgt5 1058508.682 ± 155.767 ns/op
Re: RFR: 8305157: The java.util.Arrays class should be declared final
On Wed, 29 Mar 2023 08:13:06 GMT, Per Minborg wrote: > Non-instantiable utility classes should be declared `final` and have a > private constructors. > > See Effective Java, Third Edition, Joshua Bloch (for example, Item 19 or Item > 22). > There are numerous examples in the JDK where we have the lone private constructor throw an exception to make sure the class isn't instantiated. - PR Comment: https://git.openjdk.org/jdk/pull/13221#issuecomment-1488934132
Re: RFR: 8305157: The java.util.Arrays class should be declared final
On Wed, 29 Mar 2023 08:13:06 GMT, Per Minborg wrote: > Non-instantiable utility classes should be declared `final` and have a > private constructors. > > See Effective Java, Third Edition, Joshua Bloch (for example, Item 19 or Item > 22). In some future, one could mistakenly instantiate the class from some new method in the class itself. Throwing protects against such mistakes already during testing, avoiding a reviewer to purposely checking that the empty constructor is never invoked. - PR Comment: https://git.openjdk.org/jdk/pull/13221#issuecomment-1488920408
Re: RFR: 8304976: Optimize DateTimeFormatterBuilder.ZoneTextPrinterParser.getTree() [v2]
On Wed, 29 Mar 2023 08:16:32 GMT, Sergey Tsypanov wrote:
>> 1) When `DateTimeFormatter` is reused we don't need to copy
>> `availableZoneIds` and allocate `nonRegionIds` as PrefixTree can be taken
>> from cache. In the related benchmark allocation of `HashSet` takes ~93% of
>> all time, so avoiding it should bring some improvement for cases when we
>> reuse `DateTimeFormatter` to parse a string into `ZoneDateTime`
>> 
>>
>> 2) `DateTimeFormatter` is mostly used with one locale, so `cachedTree` and
>> `cachedTreeCI` can have predefined size.
>>
>>
>> @State(Scope.Thread)
>> @BenchmarkMode(Mode.AverageTime)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> public class DateTimeFormatterBenchmark {
>>
>> private static final DateTimeFormatter df = new
>> DateTimeFormatterBuilder().appendPattern(":MM:dd:HH:mm:v").toFormatter();
>> private static final String TEXT = "2015:03:10:12:13:ECT";
>>
>> @Setup
>> public void setUp() {
>> ZonedDateTime.parse(TEXT, df);
>> }
>>
>> @Benchmark
>> public ZonedDateTime parse() {
>> return ZonedDateTime.parse(TEXT, df);
>> }
>> }
>
> Sergey Tsypanov has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 8304745: Fix package
LGTM
-
Marked as reviewed by naoto (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/12612#pullrequestreview-1363506079
Re: RFR: 8304840: Dangling `CharacterCodingException` in a few javadoc descriptions [v2]
> Fixing a documentation bug for `CharacterCodingException` without any > description to it as attached. A corresponding CSR is also drafted. > >  Naoto Sato has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains three commits: - Merge branch 'master' into JDK-8304840-CharacterCodingException - Duplicate descriptions in CharacterCodingException - 8304840: Dangling `CharacterCodingException` in a few javadoc descriptions - Changes: https://git.openjdk.org/jdk/pull/13214/files Webrev: https://webrevs.openjdk.org/?repo=jdk=13214=01 Stats: 15 lines in 2 files changed: 14 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/13214.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13214/head:pull/13214 PR: https://git.openjdk.org/jdk/pull/13214
Integrated: 8304840: Dangling `CharacterCodingException` in a few javadoc descriptions
On Tue, 28 Mar 2023 18:25:21 GMT, Naoto Sato wrote: > Fixing a documentation bug for `CharacterCodingException` without any > description to it as attached. A corresponding CSR is also drafted. > >  This pull request has now been integrated. Changeset: e3855d00 Author:Naoto Sato URL: https://git.openjdk.org/jdk/commit/e3855d005408945ea00e3bc38a0f10bef45cd627 Stats: 15 lines in 2 files changed: 14 ins; 0 del; 1 mod 8304840: Dangling `CharacterCodingException` in a few javadoc descriptions Reviewed-by: alanb, iris, rriggs, jpai - PR: https://git.openjdk.org/jdk/pull/13214
Re: RFR: 8305157: The java.util.Arrays class should be declared final
On Wed, 29 Mar 2023 16:09:18 GMT, Raffaello Giulietti wrote: > > Would it make sense to throw an `AssertionError` in the constructor? Just in > case... It hardly seems worth the bytecode; the constructor could only be invoked by breaking in using reflection and the result would be an instance with no instance methods. - PR Comment: https://git.openjdk.org/jdk/pull/13221#issuecomment-1488911733
Re: RFR: 8305157: The java.util.Arrays class should be declared final
On Wed, 29 Mar 2023 08:13:06 GMT, Per Minborg wrote: > Non-instantiable utility classes should be declared `final` and have a > private constructors. > > See Effective Java, Third Edition, Joshua Bloch (for example, Item 19 or Item > 22). The CSR implies that java.util.Arrays has a non-private constructor but it does. The CSR should mention that the constructor is private and the Array cannot be subclassed as part of the compatibility comments. - PR Comment: https://git.openjdk.org/jdk/pull/13221#issuecomment-1488909617
Re: RFR: 8298619: java/io/File/GetXSpace.java is failing [v3]
On Mon, 27 Feb 2023 22:05:20 GMT, Brian Burkhalter wrote:
>> Modify the `Space` instances used for size comparison to be created with
>> total number of bytes derived from the Windows `diskFree` utility instead of
>> Cygwin’s `df`.
>
> Brian Burkhalter 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:
>
> - 8298619: Change getSpace0() to return a boolean; print warning if total
> size is estimated instead of exact
> - Merge
> - 8298619: Load GetDiskSpaceInformationW dynamically
> - 8298619: Replace df and diskFree with native calls
> - 8298619: java/io/File/GetXSpace.java is failing
test/jdk/java/io/File/GetXSpace.java line 278:
> 276: fail(s.name() + " free space (quota)", fs, ">",
> s.size());
> 277: } else {
> 278: pass();
Can the inverted check and if/else clauses be swapped. It would read more
naturally as:
if (windows()) {
...windows code...
} else {
... non-windows code...
}
test/jdk/java/io/File/libGetXSpace.c line 66:
> 64: if (chars == NULL) {
> 65: JNU_ThrowByNameWithLastError(env, "java/lang/RuntimeException",
> 66: "GetStringChars");
Add a `return JNI_FALSE`; otherwise it falls through.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/12397#discussion_r1152181659
PR Review Comment: https://git.openjdk.org/jdk/pull/12397#discussion_r115211
Re: RFR: 8305157: The java.util.Arrays class should be declared final
On Wed, 29 Mar 2023 08:13:06 GMT, Per Minborg wrote: > Non-instantiable utility classes should be declared `final` and have a > private constructors. > > See Effective Java, Third Edition, Joshua Bloch (for example, Item 19 or Item > 22). Would it make sense to throw an `AssertionError` in the constructor? Just in case... - PR Comment: https://git.openjdk.org/jdk/pull/13221#issuecomment-1488903069
Re: RFR: 8301492: Modernize equals() method of ResourceBundle.CacheKey and Bundles.CacheKey [v3]
On Wed, 1 Feb 2023 10:36:12 GMT, Sergey Tsypanov wrote: >> `ResourceBundle.CacheKey.equals()` and `Bundles.CacheKey.equals()` are quire >> outdated. This simple clean-up modernizes them. > > Sergey Tsypanov has updated the pull request incrementally with one > additional commit since the last revision: > > Restore logic Let's wait - PR Comment: https://git.openjdk.org/jdk/pull/12328#issuecomment-1488898753
Re: RFR: JDK-8285932 Implementation of JEP 430 String Templates (Preview) [v52]
> Enhance the Java programming language with string templates, which are > similar to string literals but contain embedded expressions. A string > template is interpreted at run time by replacing each expression with the > result of evaluating that expression, possibly after further validation and > transformation. This is a [preview language feature and > API](http://openjdk.java.net/jeps/12). Jim Laskey has updated the pull request incrementally with one additional commit since the last revision: Update combine example - Changes: - all: https://git.openjdk.org/jdk/pull/10889/files - new: https://git.openjdk.org/jdk/pull/10889/files/3e1cc74a..67ffbccc Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=10889=51 - incr: https://webrevs.openjdk.org/?repo=jdk=10889=50-51 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/10889.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/10889/head:pull/10889 PR: https://git.openjdk.org/jdk/pull/10889
Re: RFR: 8304014: Convert test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java to testNG [v2]
On Wed, 29 Mar 2023 15:21:04 GMT, Lance Andersen wrote: >>> High level comment: these days we usually try to use junit 5 / jupiter >>> instead of TestNG. >> >> I think my choice of testNG here might have been influenced by other ZIP >> area tests using testNG. I guess a rewrite to junit should be pretty >> straightforward. >> >> @LanceAndersen Do you have any opinion on junit/testNG for tests like this? > >> > High level comment: these days we usually try to use junit 5 / jupiter >> > instead of TestNG. >> >> I think my choice of testNG here might have been influenced by other ZIP >> area tests using testNG. I guess a rewrite to junit should be pretty >> straightforward. >> >> @LanceAndersen Do you have any opinion on junit/testNG for tests like this? > > We have had some discussion about using junit vs testNG but we have not > mandated it so I am OK with either but certainly if you would like to move to > junit as it should be fairly straight forward for this test, I think that > would be nice > > @LanceAndersen Do you have any opinion on junit/testNG for tests like this? > > Here's a junit version for consideration: > > https://github.com/eirbjo/jdk/blob/corrupted-zip-files-ng-junit/test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java Let's go with reviewing this version, Thank you for the update Eirik - PR Comment: https://git.openjdk.org/jdk/pull/12563#issuecomment-1488838303
Re: RFR: 8304014: Convert test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java to testNG [v2]
On Wed, 29 Mar 2023 13:30:21 GMT, Eirik Bjorsnos wrote: > > High level comment: these days we usually try to use junit 5 / jupiter > > instead of TestNG. > > I think my choice of testNG here might have been influenced by other ZIP area > tests using testNG. I guess a rewrite to junit should be pretty > straightforward. > > @LanceAndersen Do you have any opinion on junit/testNG for tests like this? We have had some discussion about using junit vs testNG but we have not mandated it so I am OK with either but certainly if you would like to move to junit as it should be fairly straight forward for this test, I think that would be nice - PR Comment: https://git.openjdk.org/jdk/pull/12563#issuecomment-1488832703
Re: RFR: 8304498: JShell does not switch to raw mode when there is no /bin/test [v2]
On Tue, 28 Mar 2023 18:33:46 GMT, Andrey Turbanov wrote: > @lahodaj was the issue reported to the upstream? I've just filled: https://github.com/jline/jline3/issues/839 - PR Comment: https://git.openjdk.org/jdk/pull/13100#issuecomment-1488819982
Re: RFR: 8298619: java/io/File/GetXSpace.java is failing [v3]
On Mon, 27 Feb 2023 22:05:20 GMT, Brian Burkhalter wrote: >> Modify the `Space` instances used for size comparison to be created with >> total number of bytes derived from the Windows `diskFree` utility instead of >> Cygwin’s `df`. > > Brian Burkhalter 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: > > - 8298619: Change getSpace0() to return a boolean; print warning if total > size is estimated instead of exact > - Merge > - 8298619: Load GetDiskSpaceInformationW dynamically > - 8298619: Replace df and diskFree with native calls > - 8298619: java/io/File/GetXSpace.java is failing > This pull request has been inactive for more than 4 weeks and will be > automatically closed if another 4 weeks passes without any activity. Ping. - PR Comment: https://git.openjdk.org/jdk/pull/12397#issuecomment-1488814079
Re: RFR: 8305157: The java.util.Arrays class should be declared final
On Wed, 29 Mar 2023 09:03:58 GMT, Alan Bateman wrote: > > I thought the change would trigger a change in the public API as the > > modifiers are changed for the class. This would likely be picked up by > > compatibility checks and so, a CSR would be needed? > > It's not a compatibility issue but a CSR is needed to track the change to the > signature. Not that I disagree with this PR, but it would be interesting to see rationale for it in CSR. - PR Comment: https://git.openjdk.org/jdk/pull/13221#issuecomment-1488804341
Re: RFR: 8305157: The java.util.Arrays class should be declared final
On Wed, 29 Mar 2023 08:13:06 GMT, Per Minborg wrote: > Non-instantiable utility classes should be declared `final` and have a > private constructors. > > See Effective Java, Third Edition, Joshua Bloch (for example, Item 19 or Item > 22). Marked as reviewed by bpb (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/13221#pullrequestreview-136102
Re: RFR: JDK-8285932 Implementation of JEP 430 String Templates (Preview) [v51]
> Enhance the Java programming language with string templates, which are > similar to string literals but contain embedded expressions. A string > template is interpreted at run time by replacing each expression with the > result of evaluating that expression, possibly after further validation and > transformation. This is a [preview language feature and > API](http://openjdk.java.net/jeps/12). Jim Laskey has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 69 commits: - Merge branch 'master' into 8285932 - Update StringTemplate.combine javadoc - Requested review changes. - Clean up list construction - Review recommended changes - Tidy javadoc - Rename StringTemplate classes - Merge branch 'master' into 8285932 - Correction - Merge branch 'master' into 8285932 - ... and 59 more: https://git.openjdk.org/jdk/compare/50a995f0...3e1cc74a - Changes: https://git.openjdk.org/jdk/pull/10889/files Webrev: https://webrevs.openjdk.org/?repo=jdk=10889=50 Stats: 9255 lines in 75 files changed: 9156 ins; 24 del; 75 mod Patch: https://git.openjdk.org/jdk/pull/10889.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/10889/head:pull/10889 PR: https://git.openjdk.org/jdk/pull/10889
Integrated: 8304990: unnecessary dash in @param gives double-dash in docs
On Tue, 28 Mar 2023 21:48:23 GMT, Lance Andersen wrote: > Please review this trivial change which removes a redundant `-` from an > `@param` and cleans up the formatting for the `isValid` method. > > > It looks like there is some additional formatting clean up that can be done > but I will handle that under a separate PR. This pull request has now been integrated. Changeset: 2fa09333 Author:Lance Andersen URL: https://git.openjdk.org/jdk/commit/2fa09333ef0ac2dc1e44292f8d45d4571cb22cca Stats: 6 lines in 1 file changed: 0 ins; 1 del; 5 mod 8304990: unnecessary dash in @param gives double-dash in docs Reviewed-by: bpb, naoto - PR: https://git.openjdk.org/jdk/pull/13217
Re: RFR: 8304265: Implementation of Foreign Function and Memory API (Third Preview) [v15]
On Wed, 29 Mar 2023 08:55:08 GMT, Per Minborg wrote: >> API changes for the FFM API (third preview) >> >> Specdiff: >> https://cr.openjdk.org/~pminborg/panama/21/v1/specdiff/overview-summary.html >> >> Javadoc: >> https://cr.openjdk.org/~pminborg/panama/21/v1/javadoc/java.base/module-summary.html > > Per Minborg has updated the pull request incrementally with one additional > commit since the last revision: > > Cleanup finality Marked as reviewed by jvernee (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/13079#pullrequestreview-1363295684
Re: RFR: 8304014: Convert test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java to testNG [v2]
On Mon, 27 Feb 2023 21:13:47 GMT, Lance Andersen wrote: >> Eirik Bjorsnos 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 ten additional >> commits since the last revision: >> >> - Replace the u8, u16, u32 methods with using a little-endian ByteBuffer. >> Collapse the checkZipException and checkZipExceptionInGetInputStream into >> one method by always consuming the InputStream. Add a block comment for the >> assertZipException method. >> - Use block comments instead of javadoc comments >> - Merge branch 'master' into corrupted-zip-files-ng >> - Merge branch 'master' into corrupted-zip-files-ng >> - Give the @BeforeMethod and @AfterMethod more meaningful names >> - Improve comments and method names to help future maintainers understand >> what these tests verify. >> - Merge branch 'master' into corrupted-zip-files-ng >> - Trim whitespace and fix some spelling >> - Convert test CorruptedZipFiles to testNG and delete ZipFile constants >> copies which are now obsolete. > > Hi Eirik, > > Thank you for your suggested changes to this test. > > I think if we are going to re-work this test, we should go further including > improving the comments for future maintainers > > I made a quick pass and some initial thoughts are below > > Best > Lance > @LanceAndersen Do you have any opinion on junit/testNG for tests like this? Here's a junit version for consideration: https://github.com/eirbjo/jdk/blob/corrupted-zip-files-ng-junit/test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java - PR Comment: https://git.openjdk.org/jdk/pull/12563#issuecomment-1488678652
Re: RFR: 8305157: The java.util.Arrays class should be declared final
On Wed, 29 Mar 2023 08:13:06 GMT, Per Minborg wrote: > Non-instantiable utility classes should be declared `final` and have a > private constructors. > > See Effective Java, Third Edition, Joshua Bloch (for example, Item 19 or Item > 22). Marked as reviewed by rriggs (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/13221#pullrequestreview-1363175535
Re: RFR: 8303392: Runtime.exec and ProcessBuilder.start should use System logger [v8]
> Runtime.exec and ProcessBuilder.start methods create a new operating system > process with the program and arguments. Many applications configure a logging > subsystem to monitor application events. Logging a process start message with > the program, arguments, and stack trace can identify the caller and purpose. > Logging the process start event is complementary to the process start event > generated for JFR (Java Flight Recorder). Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Update wording to add "In the reference implementation" qualification of the logging as recommended by the CSR review. - Changes: - all: https://git.openjdk.org/jdk/pull/12862/files - new: https://git.openjdk.org/jdk/pull/12862/files/32f05be9..105eb174 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=12862=07 - incr: https://webrevs.openjdk.org/?repo=jdk=12862=06-07 Stats: 16 lines in 2 files changed: 8 ins; 0 del; 8 mod Patch: https://git.openjdk.org/jdk/pull/12862.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/12862/head:pull/12862 PR: https://git.openjdk.org/jdk/pull/12862
Re: RFR: 8304014: Convert test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java to testNG [v2]
On Wed, 29 Mar 2023 13:23:19 GMT, Daniel Fuchs wrote: > High level comment: these days we usually try to use junit 5 / jupiter > instead of TestNG. I think my choice of testNG here might have been influenced by other ZIP area tests using testNG. I guess a rewrite to junit should be pretty straightforward. @LanceAndersen Do you have any opinion on junit/testNG for tests like this? - PR Comment: https://git.openjdk.org/jdk/pull/12563#issuecomment-1488617459
Re: RFR: 8304014: Convert test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java to testNG [v2]
On Wed, 29 Mar 2023 12:19:18 GMT, Eirik Bjorsnos wrote: >> CorruptedZipFiles could benefit from some spring cleaning and a conversion >> to testNG: >> >> - The actual tests are moved into their own `@Test` methods, given more >> meaningful names and a Javadoc comment explaining the constraint being >> verified >> - The setup code is moved to a `@Before` method, slightly modernized and >> rewritten to take advantage of `assertEquals` >> - `checkZipExceptionImpl` is updated to take advantage of `expectThrows` >> - A bunch of constants copied over from `ZipFile` can be deleted since >> JDK-6225935 has long been fixed > > Eirik Bjorsnos 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 ten additional > commits since the last revision: > > - Replace the u8, u16, u32 methods with using a little-endian ByteBuffer. > Collapse the checkZipException and checkZipExceptionInGetInputStream into one > method by always consuming the InputStream. Add a block comment for the > assertZipException method. > - Use block comments instead of javadoc comments > - Merge branch 'master' into corrupted-zip-files-ng > - Merge branch 'master' into corrupted-zip-files-ng > - Give the @BeforeMethod and @AfterMethod more meaningful names > - Improve comments and method names to help future maintainers understand > what these tests verify. > - Merge branch 'master' into corrupted-zip-files-ng > - Trim whitespace and fix some spelling > - Convert test CorruptedZipFiles to testNG and delete ZipFile constants > copies which are now obsolete. High level comment: these days we usually try to use junit 5 / jupiter instead of TestNG. - PR Comment: https://git.openjdk.org/jdk/pull/12563#issuecomment-1488606123
Re: RFR: 8266571: Sequenced Collections [v4]
On Tue, 28 Mar 2023 02:37:22 GMT, Stuart Marks wrote: >> PR for Sequenced Collections implementation. > > Stuart Marks has updated the pull request incrementally with one additional > commit since the last revision: > > Simplify handling of cached keySet, values, and entrySet views. > This is taking a personal turn and I do not see the point of discussing that > matter here, so please send me an email at > [fo...@univ-mlv.fr](mailto:fo...@univ-mlv.fr) I agree we have both made our case and that a further discussion in this PR is becoming increasingly moot. - PR Comment: https://git.openjdk.org/jdk/pull/7387#issuecomment-1488595053
Re: RFR: 8266571: Sequenced Collections [v4]
On Tue, 28 Mar 2023 02:37:22 GMT, Stuart Marks wrote: >> PR for Sequenced Collections implementation. > > Stuart Marks has updated the pull request incrementally with one additional > commit since the last revision: > > Simplify handling of cached keySet, values, and entrySet views. This is taking a personal turn and I do not see the point of discussing that matter here, so please send me an email at fo...@univ-mlv.fr - PR Comment: https://git.openjdk.org/jdk/pull/7387#issuecomment-1488580845
Re: RFR: 8304014: Convert test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java to testNG [v2]
> CorruptedZipFiles could benefit from some spring cleaning and a conversion to > testNG: > > - The actual tests are moved into their own `@Test` methods, given more > meaningful names and a Javadoc comment explaining the constraint being > verified > - The setup code is moved to a `@Before` method, slightly modernized and > rewritten to take advantage of `assertEquals` > - `checkZipExceptionImpl` is updated to take advantage of `expectThrows` > - A bunch of constants copied over from `ZipFile` can be deleted since > JDK-6225935 has long been fixed Eirik Bjorsnos 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 ten additional commits since the last revision: - Replace the u8, u16, u32 methods with using a little-endian ByteBuffer. Collapse the checkZipException and checkZipExceptionInGetInputStream into one method by always consuming the InputStream. Add a block comment for the assertZipException method. - Use block comments instead of javadoc comments - Merge branch 'master' into corrupted-zip-files-ng - Merge branch 'master' into corrupted-zip-files-ng - Give the @BeforeMethod and @AfterMethod more meaningful names - Improve comments and method names to help future maintainers understand what these tests verify. - Merge branch 'master' into corrupted-zip-files-ng - Trim whitespace and fix some spelling - Convert test CorruptedZipFiles to testNG and delete ZipFile constants copies which are now obsolete. - Changes: - all: https://git.openjdk.org/jdk/pull/12563/files - new: https://git.openjdk.org/jdk/pull/12563/files/f2fd8974..e46fc5b3 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=12563=01 - incr: https://webrevs.openjdk.org/?repo=jdk=12563=00-01 Stats: 96611 lines in 1305 files changed: 56104 ins; 28357 del; 12150 mod Patch: https://git.openjdk.org/jdk/pull/12563.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/12563/head:pull/12563 PR: https://git.openjdk.org/jdk/pull/12563
Re: RFR: 8266571: Sequenced Collections [v4]
On Wed, 29 Mar 2023 11:28:39 GMT, Rémi Forax wrote: > Hi Erik, I think you misunderstood me, currently Oracle supports most of the > development of the OpenJDK financially, that's a fact and i'm glad that > Oracle has taken that mantle because I'm remembering very well the sad state > of Java at the time SUN was dying. Please do not avoid critisism by claiming to be misunderstood. This just annoys me. I'm presenting you feedback. Rather direct and tough feedback, I'll admit. Please embrace this opportunity to improve rather than drafting your defence. > I believe that introducing the interface SequencedCollection instead of > adding the methods to Collection (i get it's not the exact same semantics) > have an impact that is too big and will require more work than it should for > both the maintainers of the JDK and the maintainers of the libraries having > their own implementations of the Java collections. Then just say so. > This has nothing to do with the paycheck of some of us. Fine, then let's agree to try and avoid using that paycheck as an argument for or against the merit of any PR. - PR Comment: https://git.openjdk.org/jdk/pull/7387#issuecomment-1488481290
Re: RFR: 8266571: Sequenced Collections [v4]
On Tue, 28 Mar 2023 02:37:22 GMT, Stuart Marks wrote: >> PR for Sequenced Collections implementation. > > Stuart Marks has updated the pull request incrementally with one additional > commit since the last revision: > > Simplify handling of cached keySet, values, and entrySet views. Hi Erik, I think you misunderstood me, currently Oracle supports most of the development of the OpenJDK financially, that's a fact and i'm glad that Oracle has taken that mantle because I'm remembering very well the sad state of Java at the time SUN was dying. I believe that introducing the interface SequencedCollection instead of adding the methods to Collection (i get it's not the exact same semantics) have an impact that is too big and will require more work than it should for both the maintainers of the JDK and the maintainers of the libraries having their own implementations of the Java collections. This has nothing to do with the paycheck of some of us. - PR Comment: https://git.openjdk.org/jdk/pull/7387#issuecomment-1488423778
Re: RFR: 8303866: Allow ZipInputStream.readEnd to parse small Zip64 ZIP files [v2]
> ZipInputStream.readEnd currently assumes a Zip64 data descriptor if the > number of compressed or uncompressed bytes read from the inflater is larger > than the Zip64 magic value. > > While the ZIP format mandates that the data descriptor `SHOULD be stored in > ZIP64 format (as 8 byte values) when a file's size exceeds 0x`, it > also states that `ZIP64 format MAY be used regardless of the size of a file`. > For such small entries, the above assumption does not hold. > > This PR augments ZipInputStream.readEnd to also assume 8-byte sizes if the > ZipEntry includes a Zip64 extra information field. This brings ZipInputStream > into alignment with the APPNOTE format spec: > > > When extracting, if the zip64 extended information extra > field is present for the file the compressed and > uncompressed sizes will be 8 byte values. > > > While small Zip64 files with 8-byte data descriptors are not commonly found > in the wild, it is possible to create one using the Info-ZIP command line > `-fd` flag: > > `echo hello | zip -fd > hello.zip` > > The PR also adds a test verifying that such a small Zip64 file can be parsed > by ZipInputStream. Eirik Bjorsnos has updated the pull request incrementally with one additional commit since the last revision: Use block comments instead of javadoc comments to avoid doclint warnings - Changes: - all: https://git.openjdk.org/jdk/pull/12524/files - new: https://git.openjdk.org/jdk/pull/12524/files/0633c807..01216ef7 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=12524=01 - incr: https://webrevs.openjdk.org/?repo=jdk=12524=00-01 Stats: 5 lines in 1 file changed: 0 ins; 0 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/12524.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/12524/head:pull/12524 PR: https://git.openjdk.org/jdk/pull/12524
