Re: ReversibleCollection proposal
On 4/22/21 12:14 AM, Stephen Colebourne wrote:
public interface Collection {
default E getFirst() { return iterator().next(); }
Searching for ".iterator().next()" yields 74 matches in JDK sources...
Peter
Re: RFR: 8199594: Add doc describing how (?x) ignores spaces in character classes [v3]
> Clarifying note on comments mode to explicitly note that whitespace within > character classes is ignored. Ian Graves has updated the pull request incrementally with one additional commit since the last revision: Tweaking wording and placement - Changes: - all: https://git.openjdk.java.net/jdk/pull/3577/files - new: https://git.openjdk.java.net/jdk/pull/3577/files/f4507e3a..79390124 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3577=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3577=01-02 Stats: 11 lines in 1 file changed: 2 ins; 3 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/3577.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3577/head:pull/3577 PR: https://git.openjdk.java.net/jdk/pull/3577
Re: RFR: 8199594: Add doc describing how (?x) ignores spaces in character classes [v2]
On Wed, 21 Apr 2021 02:56:12 GMT, Stuart Marks wrote: >> Ian Graves has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Adding differences to Perl 5 note > > A few minor wording adjustments. Please update the CSR accordingly and I'll > review it too. Wording updated per @stuart-marks's suggestions. Thanks! CSR updated accordingly. - PR: https://git.openjdk.java.net/jdk/pull/3577
Re: ReversibleCollection proposal
On Wed, 21 Apr 2021 at 18:39, Stuart Marks wrote:
> The value being provided here is that the ReversibleCollection interface
> provides a
> context within which specs no longer need to hedge about "if the collection
> has an
> ordering". APIs that produce or consume ReversibleCollection no longer need to
> include this hedge, or have disclaimers about ordering. Potential new
> ReversibleCollection implementations also need not worry about this issue in
> the
> same way they did when they were forced to implement Collection directly.
Having thought about this for a few days my "interesting thought" is
that adding a `ReversibleCollection` interface and adding additional
methods can be orthogonal choices.
Specifically, I do rather like Peter's suggestion of adding more
methods to Collection. Adding these methods would be pretty useful in
general and give the proposed change much greater reach:
public interface Collection {
default void addFirst(E e) { throw new
UnsupportedOperationException(); }
default E getFirst() { return iterator().next(); }
default E removeFirst() { var i = iterator(); i.next();
i.remove(); }
}
My view being that every collection has some notion of "first", even
if that notion is "undefined". If that was the only change made, I
think that would be a good move forward.
These would be the various options:
- 7 new methods on ReversibleCollection (Stuart's suggestion)
- 7 new methods on Collection, no ReversibleCollection (Peter's suggestion)
- 3 new methods on Collection, no ReversibleCollection (my suggestion #1)
- 3 new methods on Collection, 4 methods on ReversibleCollection (my
suggestion #2)
- 7 new methods on Collection, 0 methods on ReversibleCollection (my
suggestion #3)
FWIW, I think I prefer my suggestion #2
Stephen
Re: RFR: 8203359: Container level resources events [v9]
On Wed, 21 Apr 2021 13:34:28 GMT, Jaroslav Bachorik
wrote:
>> With this change it becomes possible to surface various cgroup level metrics
>> (available via `jdk.internal.platform.Metrics`) as JFR events.
>>
>> Only a subset of the metrics exposed by `jdk.internal.platform.Metrics` is
>> turned into JFR events to start with.
>> * CPU related metrics
>> * Memory related metrics
>> * I/O related metrics
>>
>> For each of those subsystems a configuration data will be emitted as well.
>> The initial proposal is to emit the configuration data events at least once
>> per chunk and the metrics values at 30 seconds interval.
>> By using these values the emitted events seem to contain useful information
>> without increasing overhead (the metrics values are read from `/proc`
>> filesystem so that should not be done too frequently).
>
> Jaroslav Bachorik has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Fix event metadata
I wonder if something similar to below could be added to jdk.jfr.internal.Utils:
private static Metrics[] metrics;
public static Metrics getMetrics() {
if (metrics == null) {
metrics = new Metrics[] { Metrics.systemMetrics() };
}
return metrics[0];
}
public static boolean shouldSkipBytecode(String eventName, Class
superClass) {
if (superClass.getClassLoader() != null) {
return false;
}
if (!eventName.startsWith("jdk.Container")) {
return false;
}
return getMetrics() == null;
}
Then we could add checks to
jdk.jfr.internal.JVMUpcalls::bytesForEagerInstrumentation(...)
eventName = ei.getEventName();
if (Utils.shouldSkipBytecode(eventName, superClass))) {
return oldBytes;
}
and jdk.jfr.internal.JVMUpcalls:onRetransform(...)
if (jdk.internal.event.Event.class.isAssignableFrom(clazz) &&
!Modifier.isAbstract(clazz.getModifiers())) {
if (Utils.shouldSkipBytecode(clazz.getName(), clazz.getSuperclass())) {
return oldBytes;
}
This way we would not pay for generating bytecode for events in a non-container
environment.
Not sure if it works, but could perhaps make startup faster? We would still pay
for generating the event handlers during registration, but it's much trickier
to avoid since we need to store the event type somewhere.
-
PR: https://git.openjdk.java.net/jdk/pull/3126
Jpackage Mac entitlements
Reverted to Jdk 16 and temporarily couldn’t figure out why AppleScript wasn’t working again. I needed to provide an entitlement. At jpackage 16 I found I needed to do this differently with a .entitlements file in the resources directory, as I noticed in the verbose command output. I had previously noticed that jpackage 17 now includes a parameter to provide this. Will both options continue to be supported, resource dir and parm, or will parm become the only supported way at or after 17?
Re: RFR: 8265137: java.util.Random suddenly has new public methods nowhere documented [v6]
On Wed, 21 Apr 2021 13:13:16 GMT, Jim Laskey wrote: >> Move makeXXXSpilterator from public (@hidden) to protected. No API ch > > Jim Laskey 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 eight additional commits since > the last revision: > > - Merge branch 'master' into 8265137 > - Remove @hidden > - Correct the hierarchy of Random > - Remove extraneous references to makeXXXSpliterator > - Move makeXXXSpliterator methods to RandomSupport > - change static final from 'proxy' to 'PROXY' > - Make makeXXXSpliterator final > - Move makeXXXSpilterator from public (@hidden) to protected. No API change. Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3469
Integrated: 8265700: Regularize throws clauses in BigDecimal
On Wed, 21 Apr 2021 21:05:19 GMT, Joe Darcy wrote: > Some throws clauses in BigDecimal are uninformative and could be eliminated. > A few explicit throws clauses for surprising exceptions could be added. > > These are spec clarifications rather than spec changes. > > Follow-up work to https://github.com/openjdk/jdk/pull/3189. This pull request has now been integrated. Changeset: 9e7c748d Author:Joe Darcy URL: https://git.openjdk.java.net/jdk/commit/9e7c748d Stats: 39 lines in 1 file changed: 8 ins; 31 del; 0 mod 8265700: Regularize throws clauses in BigDecimal Reviewed-by: bpb - PR: https://git.openjdk.java.net/jdk/pull/3608
Re: RFR: 8265700: Regularize throws clauses in BigDecimal [v2]
On Wed, 21 Apr 2021 21:27:40 GMT, Joe Darcy wrote: >> Some throws clauses in BigDecimal are uninformative and could be eliminated. >> A few explicit throws clauses for surprising exceptions could be added. >> >> These are spec clarifications rather than spec changes. >> >> Follow-up work to https://github.com/openjdk/jdk/pull/3189. > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Include constructors. Less is more. Approved. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3608
Re: RFR: 8264161: BigDecimal#stripTrailingZeros can throw undocumented ArithmeticException
On Thu, 25 Mar 2021 08:19:24 GMT, Fabian Meumertzheim
wrote:
> Adds missing @throws declarations for ArithmeticException to the public
> function
> java.math.BigDecimal#stripTrailingZeros
> as well as the private helper functions
> java.math.BigDecimal#createAndStripZerosToMatchScale(long, int, long)
> java.math.BigDecimal#createAndStripZerosToMatchScale(BigInteger, int, long)
>
> stripTrailingZeros calls one of the two helper functions, both of which
> can repeatedly decrease the scale by 1 until it underflows. If it does,
> the call to checkScale will result in an ArithmeticException (overflow).
>
>
> > Just was we don't think it is helpful to put an explicit
> > @throws NullPointerException if a argument is null
> > on every method that throws a NPE for a null argument, I don't think it
> > is helpful or necessary to explicitly note in every BigDecimal method
> > with rounding that it could throw ArithmeticException. The general
> > class-level statement
> > ?* As a 32-bit integer, the set of values for the scale is large,
> > ?* but bounded. If the scale of a result would exceed the range of a
> > ?* 32-bit integer, either by overflow or underflow, the operation may
> > ?* throw an {@code ArithmeticException}.
> > in meant to capture the needed information.
> > -Joe
>
> I do agree with this in general, but I think that the situation at hand is a
> bit different from your example for two reasons:
>
> * The `BigDecimal` class already contains many explicit `@throws`
> annotations for `RuntimeException`s. The absence of such an annotation from a
> particular method would thus naturally be interpreted as saying that the
> method does not throw.
>
> * For someone not intimately familiar with the internal representation of
> `BigDecimal`s, it is probably quite unexpected that a function called
> `stripTrailingZeros` would perform rounding and thus be liable to scale
> overflows. (This is what happened to the maintainers of jackson-core, where
> this lead to an unexpected RuntimeException that was only discovered via
> fuzzing. They simply took this function to perform some kind of "harmless"
> canonicalization.)
>
>
> That is why I do see value in adding these annotations in this particular
> case.
Thanks for the additional context. Fuzzers would be likely to run into an
exception from stripTrailingZeros as they would be likely to start with extreme
exponent values.
Looking over BigDecimal's throws clauses more closely, I've filed
JDK-8265700: Regularize throws clauses in BigDecimal
and will send out a PR shortly. This change will add a throws clause for the
unexpected exception from stripTrailingZeros and remove several "throws
ArithmeticException when rounding occurs and the rounding mode is UNNECESSARY"
clauses.
-
PR: https://git.openjdk.java.net/jdk/pull/3189
Re: RFR: 8265700: Regularize throws clauses in BigDecimal [v2]
> Some throws clauses in BigDecimal are uninformative and could be eliminated. > A few explicit throws clauses for surprising exceptions could be added. > > These are spec clarifications rather than spec changes. > > Follow-up work to https://github.com/openjdk/jdk/pull/3189. Joe Darcy has updated the pull request incrementally with one additional commit since the last revision: Include constructors. - Changes: - all: https://git.openjdk.java.net/jdk/pull/3608/files - new: https://git.openjdk.java.net/jdk/pull/3608/files/676ea4c5..1829b08c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3608=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3608=00-01 Stats: 22 lines in 1 file changed: 1 ins; 19 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/3608.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3608/head:pull/3608 PR: https://git.openjdk.java.net/jdk/pull/3608
RFR: 8265700: Regularize throws clauses in BigDecimal
Some throws clauses in BigDecimal are uninformative and could be eliminated. A few explicit throws clauses for surprising exceptions could be added. These are spec clarifications rather than spec changes. Follow-up work to https://github.com/openjdk/jdk/pull/3189. - Commit messages: - Add missing space. - 8265700: Regularize throws clauses in BigDecimal Changes: https://git.openjdk.java.net/jdk/pull/3608/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3608=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8265700 Stats: 19 lines in 1 file changed: 7 ins; 12 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3608.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3608/head:pull/3608 PR: https://git.openjdk.java.net/jdk/pull/3608
Re: RFR: 8264514: HexFormat implementation tweaks
On Wed, 7 Apr 2021 18:49:36 GMT, Raffaello Giulietti wrote: > (Changed to new branch in personal fork) > > Please review these small tweaks. > For background information see > [1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-March/075822.html) > > Greetings > Raffaello Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3381
Re: [External] : Re: ReversibleCollection proposal
On 4/19/21 2:01 PM, Remi Forax wrote:
- Mail original -
Thinking a little bit about your proposal,
introducing an interface right in the middle of a hierarchy is not a backward
compatible change
(you have an issue when the compiler has to use the lowest upper bound).
By example
void m(List> list) { ... }
var list = List.of(new LinkedHashSet(), List.of("foo"));
m(list); // does not compile anymore
currently the type of list is List> but with your proposal, the type
will be List>
Yes, interesting. Not too difficult to fix though. Either change the method
declaration to
void m(List> list)
or change the 'var' to an explicit declaration of List>.
s'marks
Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxiliary classes [v2]
On 20/04/2021 21:26, Rafael Winterhalter wrote: I have earlier proposed to offer a "jdk.test" module that offers the Instrumentation instance via a simple API similar to Byte Buddy's. The JVM would not load this module unless requested on the command line. Build tools like Maven's surefire or Gradle's testrunner could then standardize on loading this module as a convention to give access to this test module by default such that libraries like Mockito could continue to function out of the box without the libraries functioning on a standard VM without extra configuration. As far as I know, mainly test libraries need this API. This would also emphasise that Mockito and others are meant for testing and fewer people would abuse it for production applications. People would also have an explicit means of running a JVM for a production application or for executing a test. Helping testing is good but a jdk.test module that hands out the Instrumentation object could be problematic. Is there a reason why the runner for Mockito and other mocking frameworks can't specify -javaagent when launching? I would expect at least some mocking scenarios to require load time transformations (to drop the final modifier from some API elements for example) so important to have the transformer set before classes are loaded. As for adding the API, my thought is that if the Instrumentation API were to throw exceptions on some methods/arguments for dynamic agents in the future, for example for retransformClasses(Object.class), this breaking change would then simply extend to the proposed "defineClass" method. In this sense, the Instrumentation API already assumes full power, I find it not problematic to add the missing bit to this API even if it was restricted in the future in the same spirit as other methods of the API would be. I think it would really hard to put this genie back into a bottle. It's way more attractive to use that than the very agent oriented redefineModule and retransformClasses. I mentioned JNI as it is a well-known approach to defining a class today, using a minimal native binding to an interface that directly calls down to JNI's: jclass DefineClass(JNIEnv *env, const char *name, jobject loader, const jbyte *buf, jsize bufLen); This interface can then simply be used to define any class just as I propse, even when not writing an agent or attaching. This method makes class definitions also already trivial for JVMTI agents compared to Java agents. Unless restricting JNI, the defineClass method is already a low hanging fruit, but at the cost of having to maintain a tiny bit of native code. Sure, if you are using native code then you have the full power of JVM TI and JNI available. Project Panama is exploring how to restrict access to native code, I think too early to say how this might extend to JNI. -Alan
Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxiliary classes [v2]
Hi Ron, we certainly do come from different backgrounds and therefore perspectives, I find this exchange of perspective to be the main advantage of this open mailing list. I do believe that I have an understanding of your angle, I tried to give my feedback since before Java 9 introduced the module system and so long I feel like it always helped to find a good compromise. In the end, if a library like Mockito or the large set of agents would no longer work, this would be significantly more impactful to the JVM ecosystem then any minor API change that gets carefully reviewed for good reason. That's why I am trying to preserve them. The lack of a class definition utility for Java agents is, in my opinion, one of the last bits within "unsafe migration" that has not been addressed. And it seems to me that this view is agreed upon, Oracle already proposed an API to potentially tackle this issue, I just disagree with the scope, given my extensive experience in agent development, and try to revive the hibernated debate at the same time. I am very much aware that this metaphoric fence between Java agents and regular libraries pretending to be one is being built. I also think this is necessary, it's a good step forward. And as a matter of fact, expecting this is at the core of my argument. With the fence being completed at some point, the Instrumentation instance would be shielded off from regular libraries, therefore the restriction to emulate being an agent would imply a restriction to define classes using this instance. At the same time, Java agents could use a facility that they certainly require, I hopefully lined out why this is needed. Today most Java agents are using Unsafe, but I would hope that they could migrate to an official API for Java 17. This way agents could disregard this fence and continue to function when building the fence is completed, even if a user upgrades the JVM. My second argument is that restricting dynamic agents would require to stub some API of Instrumentation for some arguments already. I do not see any (additional) harm in stubbing the defineClass API in a similar manner as it would require stubbing retransformClasses. I think it would therefore be best to keep this argument out of the consideration of what the best API would be for "favored agents", where arguments in favor of my proposal were made by not only me. Therefore, I hope this API can be considered. Agent developers would certainly want to migrate to stable, supported API. But as of today no such API is offered. Since Java agents are often supplementary and need to support unmaintained software, their baseline moves slower then that of other Java code which is why I hoped that Java 17 could be the first release to offer such API as it gets the LTS label attached. Best regards, Rafael Am Mi., 21. Apr. 2021 um 19:00 Uhr schrieb Ron Pressler < ron.press...@oracle.com>: > I think you’re coming to this discussion with a very different perspective > from us, which > makes understanding what is or isn’t on the table unclear, and also steers > the ideas in > directions that are different from the one we’re going. > > Our goal is not to maintain some status quo until such time we decide to > restrict it. We’ve > started on a long process of strengthening the platform’s integrity, both > to increase security > and to help with the ecosystem’s maintenance. Making some things that are > possible today > less “convenient” is intentional. Locked doors are less convenient than > unlocked ones, but > sometimes you can only strive to increase convenience under the hard > constraint that doors > must be locked. This is where we are: we’ve decided doors will be locked > unless explicitly > unlocked. > > But this is a process. As long as there are gaps in the garden fence — > Unsafe, self-attach, JNI — > the locks can be circumvented. Rather than fixing all those loopholes at > once, we’re doing it > gradually one at a time. This does mean that a motivated developer can > circumvent the locks up > until the point the last loophole is closed, but the hope is that, knowing > where we’re headed, library > developers will gradually accept the reality of this direction (or not > prepare their users for the coming > changes, keep using those gaps that are still open to them, and then > surprise their users when the > last of them is closed). > > Suggestions should, therefore, focus on ideas compatible with this vision. > To be more constructive > and less frustrating, the discussion should proceed under this > assumption, even if it means > accepting that some things will be less convenient than they are today. > > For example, self-attach is not the only issue. Leaking powerful > Instrumentation objects to libraries > circumvents encapsulation barriers without there being a key placed in the > lock through the command > line. That this can be circumvented today is irrelevant, as these > workarounds will be *gradually* > removed. I
Re: ReversibleCollection proposal
On 4/19/21 4:05 PM, fo...@univ-mlv.fr wrote: * There's a useful clump of semantics here, combined with sensible operations that rely on those semantics. There are a lot of places in the spec where there is hedging of the form "if the collection has an ordering, then... otherwise the results are undefined". This is unnecessary in the context of a new type. Furthermore, the new operations fit well with the new types' semantics, with no hedging necessary. You can only say that a reversible collection has an ordering. But usually you want the opposite, all collections that have an ordering are a reversible collection. But while this can be true for the implementations inside the JDK that will never be true in Java because there all already collection implementations with an ordering which do not implement ReversibleCollection. Sadly, this ship has sailed. The spec will still have to say "if the collection has an ordering", otherwise the new semantics is not a backward compatible. The value being provided here is that the ReversibleCollection interface provides a context within which specs no longer need to hedge about "if the collection has an ordering". APIs that produce or consume ReversibleCollection no longer need to include this hedge, or have disclaimers about ordering. Potential new ReversibleCollection implementations also need not worry about this issue in the same way they did when they were forced to implement Collection directly. Of course there will always be ordered collections that don't implement ReversibleCollection. (Examples of this are the Queue-but-not-Deque implementations in the JDK and 3rd party ordered collections that implement Collection directly.) Existing APIs that produce or consume Collection but have stipulations about ordering may need to remain, although some could be adjusted depending on the context. You rightly point out that this problem can never be solved in general. However, it's not a goal for ReversibleCollection to represent *all* ordered collections, so it's hardly a criticism that it doesn't solve a problem that cannot be solved. * These semantics appear across a broad range of existing collection types and implementations. It's quite valuable to have a type that unifies the common pieces of List, Deque, SortedSet, and LinkedHashSet into a single abstraction. yes in term of documentation, but in Java, it also means that you can use that interface as a type. For a List, a Deque or a Set, there are known algorithms that takes such type has parameter so it makes sense to have such type. For a ReversibleCollection, i do not see many codes that will benefit taking a ReversibleCollection as parameter instead of using Collection. It seems likely that many public APIs (both in the JDK and in external libraries) will continue to use Collection, for compatibility reasons. In certain cases (such as LinkedHashMap, see below) the APIs could be adjusted, if the compatibility impact is small or can be mitigated. ReversibleCollection unifies a broad set of existing collections without requiring any retrofitting at all. Many applications and libraries don't have their own collection implementations; they just use the ones in the JDK. They can benefit from the new APIs in ReversibleCollection immediately, without the need for any retrofitting at all. ReversibleCollection also provide opportunities for new code and for existing APIs that don't have stringent compatibility constraints. Consider an application that has a method wants to consume an ordered collection; a reasonable API would take a List as a parameter. Suppose a caller happened to have a LinkedHashSet in the right order (for example, because it wanted to deduplicate the elements). Either the caller would be forced to copy the LHS into a List before passing it, or the callee could adjust its parameter to be Collection -- but this creates the possibility of bugs if a HashSet is passed by accident. Using ReversibleCollection as a parameter type fixes this problem. * It's useful to have a new type for return types in APIs. Consider LinkedHashMap's entrySet, keySet, and values views. While we clearly get by with having these return Set or Collection today, we need a place to put the new methods. Either they go on Collection (and have extremely weak semantics) or we define new interfaces. Even if you define a new interface, you can not change the return type of LinkedHashMap entrySet because it's not a backward compatible change. LinkedHashMap is not final so you have to update all subclasses of LinkedHashMap too. As I've mentioned, introducing the covariant overrides for LinkedHashMap views is a compatibility *risk* but by itself this is not dispositive. In the past we had no way to assess this risk, so we simply avoided ever making such changes. This might be why NavigableMap introduced navigableKeySet to return a NavigableSet instead of
Re: RFR: 8264161: BigDecimal#stripTrailingZeros can throw undocumented ArithmeticException
On Wed, 21 Apr 2021 16:27:14 GMT, Joe Darcy wrote:
> Just was we don't think it is helpful to put an explicit
>
> @throws NullPointerException if a argument is null
>
> on every method that throws a NPE for a null argument, I don't think it
> is helpful or necessary to explicitly note in every BigDecimal method
> with rounding that it could throw ArithmeticException. The general
> class-level statement
>
> ?* As a 32-bit integer, the set of values for the scale is large,
> ?* but bounded. If the scale of a result would exceed the range of a
> ?* 32-bit integer, either by overflow or underflow, the operation may
> ?* throw an {@code ArithmeticException}.
>
> in meant to capture the needed information.
>
> -Joe
I do agree with this in general, but I think that the situation at hand is a
bit different from your example for two reasons:
* The `BigDecimal` class already contains many explicit `@throws` annotations
for `RuntimeException`s. The absence of such an annotation from a particular
method would thus naturally be interpreted as saying that the method does not
throw.
* For someone not intimately familiar with the internal representation of
`BigDecimal`s, it is probably quite unexpected that a function called
`stripTrailingZeros` would perform rounding and thus be liable to scale
overflows. (This is what happened to the maintainers of jackson-core, where
this lead to an unexpected RuntimeException that was only discovered via
fuzzing. They simply took this function to perform some kind of "harmless"
canonicalization.)
That is why I do see value in adding these annotations in this particular case.
-
PR: https://git.openjdk.java.net/jdk/pull/3189
Re: RFR: 8264161: BigDecimal#stripTrailingZeros can throw undocumented ArithmeticException
On 4/21/2021 9:14 AM, Fabian Meumertzheim wrote:
On Thu, 25 Mar 2021 08:19:24 GMT, Fabian Meumertzheim
wrote:
Adds missing @throws declarations for ArithmeticException to the public
function
java.math.BigDecimal#stripTrailingZeros
as well as the private helper functions
java.math.BigDecimal#createAndStripZerosToMatchScale(long, int, long)
java.math.BigDecimal#createAndStripZerosToMatchScale(BigInteger, int, long)
stripTrailingZeros calls one of the two helper functions, both of which
can repeatedly decrease the scale by 1 until it underflows. If it does,
the call to checkScale will result in an ArithmeticException (overflow).
Please note the issue
[8264161](https://bugs.openjdk.java.net/browse/JDK-8264161) has already been
addressed by:
#3204
@RogerRiggs The fix in #3204 remains incomplete as it does not update the `@throws`
declarations. I could open a new bug for that, but unfortunately the form at
https://bugreport.java.com/bugreport/start_form.do appears to be down: There is no server
response when clicking "Submit", the request to
https://bugreport.java.com/bugreport/submit_start.do just hangs forever.
Just was we don't think it is helpful to put an explicit
@throws NullPointerException if a argument is null
on every method that throws a NPE for a null argument, I don't think it
is helpful or necessary to explicitly note in every BigDecimal method
with rounding that it could throw ArithmeticException. The general
class-level statement
* As a 32-bit integer, the set of values for the scale is large,
* but bounded. If the scale of a result would exceed the range of a
* 32-bit integer, either by overflow or underflow, the operation may
* throw an {@code ArithmeticException}.
in meant to capture the needed information.
-Joe
Re: RFR: 8264161: BigDecimal#stripTrailingZeros can throw undocumented ArithmeticException
On Apr 21, 2021, at 9:23 AM, Brian Burkhalter mailto:brian.burkhal...@oracle.com>> wrote: There is in fact some system maintenance which should be completed this morning, Pacific Time. After that a new issue for this should be filed as we cannot reuse issue IDs. Correction: not sure about system maintenance (dates confused). Sorry.
Re: RFR: 8264161: BigDecimal#stripTrailingZeros can throw undocumented ArithmeticException
On Apr 21, 2021, at 9:14 AM, Fabian Meumertzheim mailto:github.com+4312191+fm...@openjdk.java.net>> wrote: On Thu, 25 Mar 2021 08:19:24 GMT, Fabian Meumertzheim mailto:github.com+4312191+fm...@openjdk.org>> wrote: Adds missing @throws declarations for ArithmeticException to the public function java.math.BigDecimal#stripTrailingZeros as well as the private helper functions java.math.BigDecimal#createAndStripZerosToMatchScale(long, int, long) java.math.BigDecimal#createAndStripZerosToMatchScale(BigInteger, int, long) stripTrailingZeros calls one of the two helper functions, both of which can repeatedly decrease the scale by 1 until it underflows. If it does, the call to checkScale will result in an ArithmeticException (overflow). Please note the issue [8264161](https://bugs.openjdk.java.net/browse/JDK-8264161) has already been addressed by: #3204 @RogerRiggs The fix in #3204 remains incomplete as it does not update the `@throws` declarations. I could open a new bug for that, but unfortunately the form at https://bugreport.java.com/bugreport/start_form.do appears to be down: There is no server response when clicking "Submit", the request to https://bugreport.java.com/bugreport/submit_start.do just hangs forever. There is in fact some system maintenance which should be completed this morning, Pacific Time. After that a new issue for this should be filed as we cannot reuse issue IDs. Thanks.
Re: RFR: 8264161: BigDecimal#stripTrailingZeros can throw undocumented ArithmeticException
On Thu, 25 Mar 2021 08:19:24 GMT, Fabian Meumertzheim wrote: > Adds missing @throws declarations for ArithmeticException to the public > function > java.math.BigDecimal#stripTrailingZeros > as well as the private helper functions > java.math.BigDecimal#createAndStripZerosToMatchScale(long, int, long) > java.math.BigDecimal#createAndStripZerosToMatchScale(BigInteger, int, long) > > stripTrailingZeros calls one of the two helper functions, both of which > can repeatedly decrease the scale by 1 until it underflows. If it does, > the call to checkScale will result in an ArithmeticException (overflow). > Please note the issue > [8264161](https://bugs.openjdk.java.net/browse/JDK-8264161) has already been > addressed by: > #3204 @RogerRiggs The fix in #3204 remains incomplete as it does not update the `@throws` declarations. I could open a new bug for that, but unfortunately the form at https://bugreport.java.com/bugreport/start_form.do appears to be down: There is no server response when clicking "Submit", the request to https://bugreport.java.com/bugreport/submit_start.do just hangs forever. - PR: https://git.openjdk.java.net/jdk/pull/3189
Re: RFR: 8264161: BigDecimal#stripTrailingZeros can throw undocumented ArithmeticException
On Thu, 25 Mar 2021 08:19:24 GMT, Fabian Meumertzheim wrote: > Adds missing @throws declarations for ArithmeticException to the public > function > java.math.BigDecimal#stripTrailingZeros > as well as the private helper functions > java.math.BigDecimal#createAndStripZerosToMatchScale(long, int, long) > java.math.BigDecimal#createAndStripZerosToMatchScale(BigInteger, int, long) > > stripTrailingZeros calls one of the two helper functions, both of which > can repeatedly decrease the scale by 1 until it underflows. If it does, > the call to checkScale will result in an ArithmeticException (overflow). Please note the issue [8264161](https://bugs.openjdk.java.net/browse/JDK-8264161) has already been addressed by: https://github.com/openjdk/jdk/pull/3204 - PR: https://git.openjdk.java.net/jdk/pull/3189
Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxiliary classes [v2]
I think you’re coming to this discussion with a very different perspective from us, which makes understanding what is or isn’t on the table unclear, and also steers the ideas in directions that are different from the one we’re going. Our goal is not to maintain some status quo until such time we decide to restrict it. We’ve started on a long process of strengthening the platform’s integrity, both to increase security and to help with the ecosystem’s maintenance. Making some things that are possible today less “convenient” is intentional. Locked doors are less convenient than unlocked ones, but sometimes you can only strive to increase convenience under the hard constraint that doors must be locked. This is where we are: we’ve decided doors will be locked unless explicitly unlocked. But this is a process. As long as there are gaps in the garden fence — Unsafe, self-attach, JNI — the locks can be circumvented. Rather than fixing all those loopholes at once, we’re doing it gradually one at a time. This does mean that a motivated developer can circumvent the locks up until the point the last loophole is closed, but the hope is that, knowing where we’re headed, library developers will gradually accept the reality of this direction (or not prepare their users for the coming changes, keep using those gaps that are still open to them, and then surprise their users when the last of them is closed). Suggestions should, therefore, focus on ideas compatible with this vision. To be more constructive and less frustrating, the discussion should proceed under this assumption, even if it means accepting that some things will be less convenient than they are today. For example, self-attach is not the only issue. Leaking powerful Instrumentation objects to libraries circumvents encapsulation barriers without there being a key placed in the lock through the command line. That this can be circumvented today is irrelevant, as these workarounds will be *gradually* removed. I hope the operations people will be thrilled with the platform’s increased security and reduced maintenance pain when upgrading JDK versions, but either way, they should start preparing for the new reality sooner rather than later. Our goal, then, should be to make people’s life easier, but only under these constraints, that, at this point, should be taken as an axiom for the purpose of discussion. — Ron > On 20 Apr 2021, at 21:26, Rafael Winterhalter > wrote: > > On Fri, 16 Apr 2021 20:30:15 GMT, Rafael Winterhalter > wrote: > >>> To allow agents the definition of auxiliary classes, an API is needed to >>> allow this. Currently, this is often achieved by using `sun.misc.Unsafe` or >>> `jdk.internal.misc.Unsafe` ever since the `defineClass` method was removed >>> from `sun.misc.Unsafe`. >> >> Rafael Winterhalter has refreshed the contents of this pull request, and >> previous commits have been removed. The incremental views will show >> differences compared to the previous content of the PR. The pull request >> contains one new commit since the last revision: >> >> 8200559: Java agents doing instrumentation need a means to define auxiliary >> classes > > I fully understand your concerns about ByteBuddyAgent.install(). It is > simply a convenience for something that can be meaningful in some contexts > where I prefer offering a simple API. I use it mainly for two purposes: > > a) For testing Java agents and integrations against Instrumentation within > the current VM when tests are triggered by tools that do not support > javaagents, also because builds do not bundle jars until after tests are > executed. > > b) For purposefully "hacky" test libraries like Mockito that need agent > capabilities without this being meant to be used in production > environments. I have earlier proposed to offer a "jdk.test" module that > offers the Instrumentation instance via a simple API similar to Byte > Buddy's. The JVM would not load this module unless requested on the command > line. Build tools like Maven's surefire or Gradle's testrunner could then > standardize on loading this module as a convention to give access to this > test module by default such that libraries like Mockito could continue to > function out of the box without the libraries functioning on a standard VM > without extra configuration. As far as I know, mainly test libraries need > this API. This would also emphasise that Mockito and others are meant for > testing and fewer people would abuse it for production applications. People > would also have an explicit means of running a JVM for a production > application or for executing a test. > > As for adding the API, my thought is that if the Instrumentation API were > to throw exceptions on some methods/arguments for dynamic agents in the > future, for example for retransformClasses(Object.class), this breaking > change would then simply extend to the proposed "defineClass" method. In > this
Integrated: 8037397: RegEx pattern matching loses character class after intersection (&&) operator
On Wed, 31 Mar 2021 20:38:33 GMT, Ian Graves wrote: > Bug fix with the intersection `&&` operator in regex patterns. In > JDK-8037397, some character classes on the right hand side of the operator > are dropped in cases where nested `[..]` classes are used with non "nested" > ones. This pull request has now been integrated. Changeset: b337f633 Author:Ian Graves Committer: Roger Riggs URL: https://git.openjdk.java.net/jdk/commit/b337f633 Stats: 41 lines in 2 files changed: 38 ins; 0 del; 3 mod 8037397: RegEx pattern matching loses character class after intersection (&&) operator Reviewed-by: rriggs - PR: https://git.openjdk.java.net/jdk/pull/3291
Re: RFR: 8263668: Update java.time to use instanceof pattern variable [v7]
On Wed, 21 Apr 2021 11:06:16 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.time` >> package to make use of the `instanceof` pattern variable? >> >> Kind regards, >> Patrick > > Patrick Concannon 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 10 additional > commits since the last revision: > > - Updated single letter pattern variable name in java/time/Duration > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Updated single letter pattern variable names > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - 8263668: Update java.time to use instanceof pattern variable Marked as reviewed by chegar (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3170
RFR: 8264161: BigDecimal#stripTrailingZeros can throw undocumented ArithmeticException
Adds missing @throws declarations for ArithmeticException to the public function java.math.BigDecimal#stripTrailingZeros as well as the private helper functions java.math.BigDecimal#createAndStripZerosToMatchScale(long, int, long) java.math.BigDecimal#createAndStripZerosToMatchScale(BigInteger, int, long) stripTrailingZeros calls one of the two helper functions, both of which can repeatedly decrease the scale by 1 until it underflows. If it does, the call to checkScale will result in an ArithmeticException (overflow). - Commit messages: - 8264161: ArithmeticException not declared for BigDecimal#stripTrailingZeros Changes: https://git.openjdk.java.net/jdk/pull/3189/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3189=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8264161 Stats: 3 lines in 1 file changed: 3 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3189.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3189/head:pull/3189 PR: https://git.openjdk.java.net/jdk/pull/3189
Re: RFR: 8264161: BigDecimal#stripTrailingZeros can throw undocumented ArithmeticException
On Thu, 25 Mar 2021 08:19:24 GMT, Fabian Meumertzheim wrote: > Adds missing @throws declarations for ArithmeticException to the public > function > java.math.BigDecimal#stripTrailingZeros > as well as the private helper functions > java.math.BigDecimal#createAndStripZerosToMatchScale(long, int, long) > java.math.BigDecimal#createAndStripZerosToMatchScale(BigInteger, int, long) > > stripTrailingZeros calls one of the two helper functions, both of which > can repeatedly decrease the scale by 1 until it underflows. If it does, > the call to checkScale will result in an ArithmeticException (overflow). Hi, please send me an e-mail at dalibor.to...@oracle.com so that I can verify your account. - PR: https://git.openjdk.java.net/jdk/pull/3189
Re: RFR: 8263668: Update java.time to use instanceof pattern variable [v7]
On Wed, 21 Apr 2021 11:06:16 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.time` >> package to make use of the `instanceof` pattern variable? >> >> Kind regards, >> Patrick > > Patrick Concannon 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 10 additional > commits since the last revision: > > - Updated single letter pattern variable name in java/time/Duration > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Updated single letter pattern variable names > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - 8263668: Update java.time to use instanceof pattern variable Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3170
Re: RFR: 8260517: implement Sealed Classes as a standard feature in Java [v3]
On Wed, 21 Apr 2021 14:42:39 GMT, Maurizio Cimadamore wrote: > Compiler changes look good (I have not checked SymbolGenerator). > > Why were some tests removed? thanks for the review. The removed tests were already covered in langtools regression tests, so I only removed duplicated tests - PR: https://git.openjdk.java.net/jdk/pull/3526
Re: RFR: 8263668: Update java.time to use instanceof pattern variable [v7]
On Wed, 21 Apr 2021 11:06:16 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.time` >> package to make use of the `instanceof` pattern variable? >> >> Kind regards, >> Patrick > > Patrick Concannon 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 10 additional > commits since the last revision: > > - Updated single letter pattern variable name in java/time/Duration > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Updated single letter pattern variable names > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - 8263668: Update java.time to use instanceof pattern variable Marked as reviewed by scolebourne (Author). - PR: https://git.openjdk.java.net/jdk/pull/3170
Re: RFR: 5023614: UUID needs methods to get most/leastSigBits and write to DataOutput [v3]
On Sat, 28 Nov 2020 10:12:10 GMT, Richard Fussenegger wrote: >> Made byte constructor public and changed the length assertion to an >> `IllegalArgumentException`, added a `getBytes` method that allows users to >> retrieve the raw bytes of the UUID, and created a new private constructor >> with an optimized construction for byte arrays that can set the version as >> desired and the variant to RFC 4122. Also changed the existing static >> factory methods to use the new constructor and removed the duplicate code >> from them where the variant and version is being set. >> >> Report >> [5023614](https://bugs.java.com/bugdatabase/view_bug.do?bug_id=5023614) asks >> for more than what I provided and with different names. However, I believe >> that there is no value in providing methods to deal with `DataInput` and >> `DataOutput` because they would only encapsulate single method calls that >> the caller can directly write as well (e.g. `output.write(uuid.getBytes())` >> vs `uuid.write(output)`). Hence, I consider this change to satisfy the >> feature request. > > Richard Fussenegger has refreshed the contents of this pull request, and > previous commits have been removed. The incremental views will show > differences compared to the previous content of the PR. Active - PR: https://git.openjdk.java.net/jdk/pull/1465
Re: RFR: 6594730: UUID.getVersion() is only meaningful for Leach-Salz variant
On Thu, 26 Nov 2020 18:51:10 GMT, Richard Fussenegger wrote: > 6594730: UUID.getVersion() is only meaningful for Leach-Salz variant Active - PR: https://git.openjdk.java.net/jdk/pull/1467
Re: RFR: 8260517: implement Sealed Classes as a standard feature in Java [v3]
On Fri, 16 Apr 2021 03:35:06 GMT, Vicente Romero wrote: >> Please review this PR that intents to make sealed classes a final feature in >> Java. This PR contains compiler and VM changes. In line with similar PRs, >> which has made preview features final, this one is mostly removing preview >> related comments from APIs plus options in test cases. Please also review >> the related [CSR](https://bugs.openjdk.java.net/browse/JDK-8265090) >> >> Thanks >> >> note: this PR is related to >> [PR-3528](https://github.com/openjdk/jdk/pull/3528) and must be integrated >> after it. > > Vicente Romero has updated the pull request incrementally with one additional > commit since the last revision: > > updating comment after review feedback Compiler changes look good (I have not checked SymbolGenerator). Why were some tests removed? - Marked as reviewed by mcimadamore (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3526
Re: RFR: 8263668: Update java.time to use instanceof pattern variable [v7]
On Wed, 21 Apr 2021 13:02:06 GMT, Andrey Turbanov wrote: > I was able to find (with IntelliJ IDEA help) few more places to improve > > ``` > java.time 27 warnings > class Clock 2 warnings > class FixedClock 1 warning > method equals(Object) 1 warning > WARNING Variable 'other' can be replaced with pattern > variable > class OffsetClock 1 warning > method equals(Object) 1 warning > WARNING Variable 'other' can be replaced with pattern > variable > class Instant 2 warnings > method until(Temporal, TemporalUnit) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > method with(TemporalField, long) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > class LocalDate 5 warnings > method minus(TemporalAmount) 1 warning > WARNING Variable 'periodToSubtract' can be replaced with > pattern variable > method plus(long, TemporalUnit) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > method plus(TemporalAmount) 1 warning > WARNING Variable 'periodToAdd' can be replaced with pattern > variable > method range(TemporalField) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > method with(TemporalField, long) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > class LocalDateTime 8 warnings > method get(TemporalField) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > method getLong(TemporalField) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > method isSupported(TemporalField) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > method minus(TemporalAmount) 1 warning > WARNING Variable 'periodToSubtract' can be replaced with > pattern variable > method plus(long, TemporalUnit) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > method plus(TemporalAmount) 1 warning > WARNING Variable 'periodToAdd' can be replaced with pattern > variable > method range(TemporalField) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > method with(TemporalField, long) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > class LocalTime 1 warning > method with(TemporalField, long) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > class OffsetDateTime 1 warning > method with(TemporalField, long) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > class Year 1 warning > method with(TemporalField, long) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > class YearMonth 1 warning > method with(TemporalField, long) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > class ZonedDateTime 6 warnings > method equals(Object) 1 warning > WARNING Variable 'other' can be replaced with pattern > variable > method minus(TemporalAmount) 1 warning > WARNING Variable 'periodToSubtract' can be replaced with > pattern variable > method plus(TemporalAmount) 1 warning > WARNING Variable 'periodToAdd' can be replaced with pattern > variable > method with(TemporalAdjuster) 2 warnings > WARNING Variable 'odt' can be replaced with pattern variable > WARNING Variable 'instant' can be replaced with pattern > variable > method with(TemporalField, long) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > java.time.chrono 13 warnings > class ChronoLocalDateImpl 1 warning > method plus(long, TemporalUnit) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > class ChronoLocalDateTimeImpl 6 warnings > method get(TemporalField) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > method getLong(TemporalField) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > method isSupported(TemporalField) 1 warning > WARNING Variable 'f' can
Re: RFR: 8263668: Update java.time to use instanceof pattern variable [v7]
On Wed, 21 Apr 2021 11:06:16 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.time` >> package to make use of the `instanceof` pattern variable? >> >> Kind regards, >> Patrick > > Patrick Concannon 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 10 additional > commits since the last revision: > > - Updated single letter pattern variable name in java/time/Duration > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Updated single letter pattern variable names > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - 8263668: Update java.time to use instanceof pattern variable Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3170
Withdrawn: 8257733: Move module-specific data from make to respective module
On Thu, 3 Dec 2020 23:44:20 GMT, Magnus Ihse Bursie wrote: > A lot (but not all) of the data in make/data is tied to a specific module. > For instance, the publicsuffixlist is used by java.base, and fontconfig by > java.desktop. (A few directories, like mainmanifest, is *actually* used by > make for the whole build.) > > These data files should move to the module they belong to. The are, after > all, "source code" for that module that is "compiler" into resulting > deliverables, for that module. (But the "source code" language is not Java or > C, but typically a highly domain specific language or data format, and the > "compilation" is, often, a specialized transformation.) > > This misplacement of the data directory is most visible at code review time. > When such data is changed, most of the time build-dev (or the new build > label) is involved, even though this has nothing to do with the build. While > this is annoying, a worse problem is if the actual team that needs to review > the patch (i.e., the team owning the module) is missed in the review. > > ### Modules reviewed > > - [x] java.base > - [x] java.desktop > - [x] jdk.compiler > - [x] java.se This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8203359: Container level resources events [v7]
On Wed, 21 Apr 2021 11:49:10 GMT, Erik Gahlin wrote: >> Ok. So what would be a good rule-of-thumb for when one should introduce a >> flag? > > I think we want to limit the number flags/options There are already too many, > preferably we would have none, but some of the ones we have today, like gc > and exception, are necessary, because the impact of have them on by default > would be catastrophic (long stop the world pauses and possibly million of > events per second). Thanks! Removed the flag. - PR: https://git.openjdk.java.net/jdk/pull/3126
Re: RFR: 8203359: Container level resources events [v8]
On Wed, 21 Apr 2021 13:21:32 GMT, Jaroslav Bachorik wrote: >> How does it look in proc? > > This was based on > https://github.com/openjdk/jdk/blob/da860290c2657c0fb1de8c77c8dffdb35f1cf938/src/java.base/linux/classes/jdk/internal/platform/CgroupV1Metrics.java#L149 > > However, that metric is 'internal' only so it really does not make sense to > keep it here and is there only by mistake. Removed - PR: https://git.openjdk.java.net/jdk/pull/3126
Re: RFR: 8203359: Container level resources events [v8]
On Wed, 21 Apr 2021 11:50:25 GMT, Erik Gahlin wrote: >> I guess we could fit those events under `Operating System/Memory` and >> `Operating System/Processor` categories. >> What about I/O? Currently, there is only `Operating System/Network` >> category. The options are: >> * Add `Operating System/IO` category and move `Network` to `Operating >> System/IO/Network` >> * Add `Operation System/FileSystem` category and move the container IO event >> there >> >> What would you prefer? > > I prefer adding File System (or having separate container category). Added `Operating System/File System` category - PR: https://git.openjdk.java.net/jdk/pull/3126
Re: RFR: 8203359: Container level resources events [v8]
On Wed, 14 Apr 2021 10:26:44 GMT, Erik Gahlin wrote:
>> Jaroslav Bachorik has updated the pull request with a new target base due to
>> a merge or a rebase. The pull request now contains 11 commits:
>>
>> - Roll back conditional registration of container events
>> - Remove container events flag
>> - Remove trailing spaces
>> - Doh
>> - Report container type and register events conditionally
>> - Remove unused test files
>> - Initial test support for JFR container events
>> - Update the JFR control files
>> - Split off the CPU throttling metrics
>> - Formatting spaces
>> - ... and 1 more:
>> https://git.openjdk.java.net/jdk/compare/e80012ed...67a61bd7
>
> src/jdk.jfr/share/classes/jdk/jfr/events/ContainerCPUThrottlingEvent.java
> line 46:
>
>> 44: public class ContainerCPUThrottlingEvent extends AbstractJDKEvent {
>> 45: @Label("CPU Elapsed Slices")
>> 46: @Description("Number of time-slice periods that have elapsed if a CPU
>> quota has been setup for the container.")
>
> If the description is one sentence, period should not be included.
Fixed in all locations
> src/jdk.jfr/share/classes/jdk/jfr/events/ContainerCPUUsageEvent.java line 46:
>
>> 44: public class ContainerCPUUsageEvent extends AbstractJDKEvent {
>> 45: @Label("CPU Time")
>> 46: @Description("Aggregate time, in nanoseconds, consumed by all tasks in
>> the container.")
>
> We usually skip the unit "nanoseconds" in descriptions when the field has a
> content type that describes the unit.
Gone
> src/jdk.jfr/share/classes/jdk/jfr/events/ContainerConfigurationEvent.java
> line 45:
>
>> 43: @Description("A set of container specific attributes.")
>> 44: public final class ContainerConfigurationEvent extends AbstractJDKEvent {
>> 45: @Label("Container type")
>
> Capitalize "type" in the label
Done
> src/jdk.jfr/share/classes/jdk/jfr/events/ContainerConfigurationEvent.java
> line 78:
>
>> 76:
>> 77: @Label("Memory and Swap Limit")
>> 78: @Description("Maximum amount of physical memory and swap space, in
>> bytes, that can be allocated in the container.")
>
> No need to mention bytes in the description when the field has DataAmount
> annotation.
Yep. Done.
> src/jdk.jfr/share/classes/jdk/jfr/events/ContainerIOUsageEvent.java line 47:
>
>> 45: public class ContainerIOUsageEvent extends AbstractJDKEvent {
>> 46:
>> 47: @Label("BlkIO Request Count")
>
> Change to "Block IO"
Done
-
PR: https://git.openjdk.java.net/jdk/pull/3126
Re: RFR: 8203359: Container level resources events [v9]
> With this change it becomes possible to surface various cgroup level metrics > (available via `jdk.internal.platform.Metrics`) as JFR events. > > Only a subset of the metrics exposed by `jdk.internal.platform.Metrics` is > turned into JFR events to start with. > * CPU related metrics > * Memory related metrics > * I/O related metrics > > For each of those subsystems a configuration data will be emitted as well. > The initial proposal is to emit the configuration data events at least once > per chunk and the metrics values at 30 seconds interval. > By using these values the emitted events seem to contain useful information > without increasing overhead (the metrics values are read from `/proc` > filesystem so that should not be done too frequently). Jaroslav Bachorik has updated the pull request incrementally with one additional commit since the last revision: Fix event metadata - Changes: - all: https://git.openjdk.java.net/jdk/pull/3126/files - new: https://git.openjdk.java.net/jdk/pull/3126/files/67a61bd7..f766faf0 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3126=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3126=07-08 Stats: 36 lines in 5 files changed: 0 ins; 5 del; 31 mod Patch: https://git.openjdk.java.net/jdk/pull/3126.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3126/head:pull/3126 PR: https://git.openjdk.java.net/jdk/pull/3126
Re: RFR: 8203359: Container level resources events [v8]
On Wed, 21 Apr 2021 11:55:48 GMT, Erik Gahlin wrote: >> I don't see this event field being set anywhere? Which Metrics API call is >> this using? > > How does it look in proc? This was based on https://github.com/openjdk/jdk/blob/da860290c2657c0fb1de8c77c8dffdb35f1cf938/src/java.base/linux/classes/jdk/internal/platform/CgroupV1Metrics.java#L149 However, that metric is 'internal' only so it really does not make sense to keep it here and is there only by mistake. - PR: https://git.openjdk.java.net/jdk/pull/3126
Re: RFR: 8265137: java.util.Random suddenly has new public methods nowhere documented [v6]
> Move makeXXXSpilterator from public (@hidden) to protected. No API ch Jim Laskey 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 eight additional commits since the last revision: - Merge branch 'master' into 8265137 - Remove @hidden - Correct the hierarchy of Random - Remove extraneous references to makeXXXSpliterator - Move makeXXXSpliterator methods to RandomSupport - change static final from 'proxy' to 'PROXY' - Make makeXXXSpliterator final - Move makeXXXSpilterator from public (@hidden) to protected. No API change. - Changes: - all: https://git.openjdk.java.net/jdk/pull/3469/files - new: https://git.openjdk.java.net/jdk/pull/3469/files/d72575d5..3381f008 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3469=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3469=04-05 Stats: 39799 lines in 1366 files changed: 8333 ins; 26676 del; 4790 mod Patch: https://git.openjdk.java.net/jdk/pull/3469.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3469/head:pull/3469 PR: https://git.openjdk.java.net/jdk/pull/3469
Re: RFR: 8263668: Update java.time to use instanceof pattern variable [v7]
On Wed, 21 Apr 2021 11:06:16 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.time` >> package to make use of the `instanceof` pattern variable? >> >> Kind regards, >> Patrick > > Patrick Concannon 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 10 additional > commits since the last revision: > > - Updated single letter pattern variable name in java/time/Duration > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Updated single letter pattern variable names > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - 8263668: Update java.time to use instanceof pattern variable I was able to find (with IntelliJ IDEA help) few more places to improve java.time 27 warnings class Clock 2 warnings class FixedClock 1 warning method equals(Object) 1 warning WARNING Variable 'other' can be replaced with pattern variable class OffsetClock 1 warning method equals(Object) 1 warning WARNING Variable 'other' can be replaced with pattern variable class Instant 2 warnings method until(Temporal, TemporalUnit) 1 warning WARNING Variable 'f' can be replaced with pattern variable method with(TemporalField, long) 1 warning WARNING Variable 'f' can be replaced with pattern variable class LocalDate 5 warnings method minus(TemporalAmount) 1 warning WARNING Variable 'periodToSubtract' can be replaced with pattern variable method plus(long, TemporalUnit) 1 warning WARNING Variable 'f' can be replaced with pattern variable method plus(TemporalAmount) 1 warning WARNING Variable 'periodToAdd' can be replaced with pattern variable method range(TemporalField) 1 warning WARNING Variable 'f' can be replaced with pattern variable method with(TemporalField, long) 1 warning WARNING Variable 'f' can be replaced with pattern variable class LocalDateTime 8 warnings method get(TemporalField) 1 warning WARNING Variable 'f' can be replaced with pattern variable method getLong(TemporalField) 1 warning WARNING Variable 'f' can be replaced with pattern variable method isSupported(TemporalField) 1 warning WARNING Variable 'f' can be replaced with pattern variable method minus(TemporalAmount) 1 warning WARNING Variable 'periodToSubtract' can be replaced with pattern variable method plus(long, TemporalUnit) 1 warning WARNING Variable 'f' can be replaced with pattern variable method plus(TemporalAmount) 1 warning WARNING Variable 'periodToAdd' can be replaced with pattern variable method range(TemporalField) 1 warning WARNING Variable 'f' can be replaced with pattern variable method with(TemporalField, long) 1 warning WARNING Variable 'f' can be replaced with pattern variable class LocalTime 1 warning method with(TemporalField, long) 1 warning WARNING Variable 'f' can be replaced with pattern variable class OffsetDateTime 1 warning method with(TemporalField, long) 1 warning WARNING Variable 'f' can be replaced with pattern variable class Year 1 warning method with(TemporalField, long) 1 warning WARNING Variable 'f' can be replaced with pattern variable class YearMonth 1 warning method with(TemporalField, long) 1 warning WARNING Variable 'f' can be replaced with pattern variable class ZonedDateTime 6 warnings method equals(Object) 1 warning WARNING Variable 'other' can be replaced with pattern variable method minus(TemporalAmount) 1 warning WARNING Variable 'periodToSubtract' can be replaced with pattern variable method plus(TemporalAmount) 1 warning WARNING Variable 'periodToAdd' can be replaced with pattern variable method with(TemporalAdjuster)
Withdrawn: 8013527: calling MethodHandles.lookup on itself leads to errors
On Wed, 3 Feb 2021 01:50:36 GMT, Mandy Chung wrote: > JDK-8013527: calling MethodHandles.lookup on itself leads to errors > JDK-8257874: MethodHandle injected invoker doesn't have necessary private > access > > Johannes Kuhn is also a contributor to this patch. > > A caller-sensitive method can behave differently depending on the identity > of its immediate caller. If a method handle for a caller-sensitive method is > requested, this resulting method handle behaves as if it were called from an > instruction contained in the lookup class. The current implementation injects > a trampoline class (aka the invoker class) which is the caller class invoking > such caller-sensitive method handle. It works in all CSMs except > `MethodHandles::lookup` > because the caller-sensitive behavior depends on the module of the caller > class, > the class loader of the caller class, the accessibility of the caller class, > or > the protection domain of the caller class. The invoker class is a hidden > class > defined in the same runtime package with the same protection domain as the > lookup class, which is why the current implementation works for all CSMs > except > `MethodHandles::lookup` which uses the caller class as the lookup class. > > Two issues with current implementation: > 1. The invoker class only has the package access as the lookup class. It > cannot > access private members of the lookup class and its nest members. > > The fix is to make the invoker class as a nestmate of the lookup class. > > 2. `MethodHandles::lookup` if invoked via a method handle produces a `Lookup` > object of an injected invoker class which is a bug. > > There are two alternatives: > - define the invoker class with the lookup class as the class data such that > `MethodHandles::lookup` will get the caller class from the class data if > it's the injected invoker > - Johannes' proposal [1]: detect if an alternate implementation with an > additional > trailing caller class parameter is present, use the alternate implementation > and bind the method handle with the lookup class as the caller class > argument. > > There has been several discussions on the improvement to support caller > sensitive > methods for example the calling sequences and security implication. I have > looked at how each CSM uses the caller class. The second approach (i.e. > defining an alternate implementation for a caller-sensitive method taking > an additional caller class parameter) does not work for non-static non-final > caller-sensitive method. In addition, it is not ideal to pollute the source > code to provide an alternatve implementation for all 120+ caller-sensitive > methods > whereas the injected invoker works for all except `MethodHandles::lookup`. > > I propose to use both approaches. We can add an alternative implementation > for > a caller-sensitive method when desirable such as `MethodHandles::lookup` in > this PR. For the injected invoker case, one could extract the original lookup > class from class data if needed. > > test/jdk/jdk/internal/reflect/CallerSensitive/CheckCSM.java ensures that > no new non-static non-final caller-sensitive method will be added to the JDK. > I extend this test to catch that non-static non-final caller-sensitive method > cannot have the alternate implementation taking the additional caller class > parameter. > > This fix for JDK-8013527 is needed by the prototype for JDK-6824466 I'm > working on. > > [1] > https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-January/073184.html This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk/pull/2367
Re: RFR: 8203359: Container level resources events [v8]
On Wed, 14 Apr 2021 14:32:32 GMT, Severin Gehwolf wrote: >> This is taken as reported by cgroups - I didn't want to change the semantics >> so it does not confuse people familiar with cgroups. > > I don't see this event field being set anywhere? Which Metrics API call is > this using? How does it look in proc? - PR: https://git.openjdk.java.net/jdk/pull/3126
Re: RFR: 8203359: Container level resources events [v7]
On Wed, 14 Apr 2021 08:28:01 GMT, Jaroslav Bachorik wrote: >> src/jdk.jfr/share/conf/jfr/default.jfc line 1051: >> >>> 1049: false >>> 1050: >>> 1051: true >> >> I don't think we should create "flag" for "Container Events". Instead we >> should treat them like CPU and other OS events, always on. Since JFR can be >> used outside a container, it seems wrong to have this as an option. > > Ok. So what would be a good rule-of-thumb for when one should introduce a > flag? I think we want to limit the number flags/options There are already too many, preferably we would have none, but some of the ones we have today, like gc and exception, are necessary, because the impact of have them on by default would be catastrophic (long stop the world pauses and possibly million of events per second). - PR: https://git.openjdk.java.net/jdk/pull/3126
Re: RFR: 8203359: Container level resources events [v8]
On Wed, 14 Apr 2021 12:14:33 GMT, Jaroslav Bachorik
wrote:
>> src/jdk.jfr/share/classes/jdk/jfr/events/ContainerCPUThrottlingEvent.java
>> line 44:
>>
>>> 42: @Category({"Operating System", "Container", "Processor"})
>>> 43: @Description("Container CPU throttling related information.")
>>> 44: public class ContainerCPUThrottlingEvent extends AbstractJDKEvent {
>>
>> I wonder if we should put all the container events in the same category
>> {"Operating System", "Container"}, Or possibly add them under the already
>> existing categories under "Operating System"?
>
> I guess we could fit those events under `Operating System/Memory` and
> `Operating System/Processor` categories.
> What about I/O? Currently, there is only `Operating System/Network` category.
> The options are:
> * Add `Operating System/IO` category and move `Network` to `Operating
> System/IO/Network`
> * Add `Operation System/FileSystem` category and move the container IO event
> there
>
> What would you prefer?
I prefer adding File System (or having separate container category).
-
PR: https://git.openjdk.java.net/jdk/pull/3126
Re: RFR: 8263668: Update java.time to use instanceof pattern variable [v7]
On Wed, 21 Apr 2021 11:06:16 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.time` >> package to make use of the `instanceof` pattern variable? >> >> Kind regards, >> Patrick > > Patrick Concannon 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 10 additional > commits since the last revision: > > - Updated single letter pattern variable name in java/time/Duration > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Updated single letter pattern variable names > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - Merge remote-tracking branch 'origin/master' into JDK-8263668 > - 8263668: Update java.time to use instanceof pattern variable Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3170
Re: RFR: 8263668: Update java.time to use instanceof pattern variable [v6]
On Wed, 21 Apr 2021 09:06:45 GMT, Daniel Fuchs wrote:
>> Patrick Concannon has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Updated single letter pattern variable names
>
> src/java.base/share/classes/java/time/Duration.java line 723:
>
>> 721: }
>> 722: if (unit instanceof ChronoUnit u) {
>> 723: switch (u) {
>
> Maybe `u` could be replaced with `chronoUnit` here too
Variable name updated as suggested in 3dc1e07
-
PR: https://git.openjdk.java.net/jdk/pull/3170
Re: RFR: 8263668: Update java.time to use instanceof pattern variable [v7]
> Hi, > > Could someone please review my code for updating the code in the `java.time` > package to make use of the `instanceof` pattern variable? > > Kind regards, > Patrick Patrick Concannon 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 10 additional commits since the last revision: - Updated single letter pattern variable name in java/time/Duration - Merge remote-tracking branch 'origin/master' into JDK-8263668 - Updated single letter pattern variable names - Merge remote-tracking branch 'origin/master' into JDK-8263668 - Merge remote-tracking branch 'origin/master' into JDK-8263668 - Merge remote-tracking branch 'origin/master' into JDK-8263668 - Merge remote-tracking branch 'origin/master' into JDK-8263668 - Merge remote-tracking branch 'origin/master' into JDK-8263668 - Merge remote-tracking branch 'origin/master' into JDK-8263668 - 8263668: Update java.time to use instanceof pattern variable - Changes: - all: https://git.openjdk.java.net/jdk/pull/3170/files - new: https://git.openjdk.java.net/jdk/pull/3170/files/647bd6b1..3dc1e075 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3170=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3170=05-06 Stats: 2549 lines in 111 files changed: 1748 ins; 362 del; 439 mod Patch: https://git.openjdk.java.net/jdk/pull/3170.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3170/head:pull/3170 PR: https://git.openjdk.java.net/jdk/pull/3170
Re: RFR: 8263668: Update java.time to use instanceof pattern variable [v6]
On Wed, 24 Mar 2021 11:06:38 GMT, Rémi Forax
wrote:
>> Patrick Concannon has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Updated single letter pattern variable names
>
> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java
> line 168:
>
>> 166: private static final TemporalQuery QUERY_REGION_ONLY =
>> (temporal) -> {
>> 167: ZoneId zone = temporal.query(TemporalQueries.zoneId());
>> 168: return (zone != null && (!(zone instanceof ZoneOffset)) ? zone
>> : null);
>
> i find this code hard to read
> `return (zone != null && (!(zone instanceof ZoneOffset))) ? zone : null;`
> seems better`
Updated in 647bd6b as suggested by Michael Kuhlmann
-
PR: https://git.openjdk.java.net/jdk/pull/3170
Integrated: 8265237: String.join and StringJoiner can be improved further
On Wed, 14 Apr 2021 18:58:57 GMT, Peter Levart wrote: > While JDK-8148937 improved StringJoiner class by replacing internal use of > getChars that copies out characters from String elements into a char[] array > with StringBuilder which is somehow more optimal, the improvement was > marginal in speed (0% ... 10%) and mainly for smaller strings, while GC was > reduced by about 50% in average per operation. > Initial attempt to tackle that issue was more involved, but was later > discarded because it was apparently using too much internal String details in > code that lives outside String and outside java.lang package. > But there is another way to package such "intimate" code - we can put it into > String itself and just call it from StringJoiner. > This PR is an attempt at doing just that. It introduces new package-private > method in `java.lang.String` which is then used from both pubic static > `String.join` methods as well as from `java.util.StringJoiner` (via > SharedSecrets). The improvements can be seen by running the following JMH > benchmark: > > https://gist.github.com/plevart/86ac7fc6d4541dbc08256cde544019ce > > The comparative results are here: > > https://jmh.morethan.io/?gist=7eb421cf7982456a2962269137f71c15 > > The jmh-result.json files are here: > > https://gist.github.com/plevart/7eb421cf7982456a2962269137f71c15 > > Improvement in speed ranges from 8% (for small strings) to 200% (for long > strings), while creation of garbage has been further reduced to an almost > garbage-free operation. > > So WDYT? This pull request has now been integrated. Changeset: 98cb81b3 Author:Peter Levart URL: https://git.openjdk.java.net/jdk/commit/98cb81b3 Stats: 214 lines in 6 files changed: 174 ins; 17 del; 23 mod 8265237: String.join and StringJoiner can be improved further Reviewed-by: rriggs, redestad - PR: https://git.openjdk.java.net/jdk/pull/3501
Re: RFR: 8263668: Update java.time to use instanceof pattern variable [v6]
On Tue, 20 Apr 2021 17:46:38 GMT, Patrick Concannon
wrote:
>> Hi,
>>
>> Could someone please review my code for updating the code in the `java.time`
>> package to make use of the `instanceof` pattern variable?
>>
>> Kind regards,
>> Patrick
>
> Patrick Concannon has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Updated single letter pattern variable names
src/java.base/share/classes/java/time/Duration.java line 723:
> 721: }
> 722: if (unit instanceof ChronoUnit u) {
> 723: switch (u) {
Maybe `u` could be replaced with `chronoUnit` here too
-
PR: https://git.openjdk.java.net/jdk/pull/3170
Integrated: 8265421: java/lang/String/StringRepeat.java test is missing a memory requirement
On Mon, 19 Apr 2021 09:42:09 GMT, Christoph Göttschkes wrote: > Please review the following patch, which fixes the mentioned test case for > memory constrained devices. This can be tested on a linux based development > machine, using systemd as follows: > > > $ systemd-run --user --scope -p MemoryMax=800M -p MemorySwapMax=0 > /usr/bin/make TEST="test/jdk/java/lang/String/StringRepeat.java" run-test > > > I split up the test case in a part which only executes the small repeat > counts, and in one part which executes the huge repeat count. The second part > is guarded by a requirement, so it is only executed if enough memory is > available. This pull request has now been integrated. Changeset: 7146104f Author:Christoph Göttschkes Committer: Aleksey Shipilev URL: https://git.openjdk.java.net/jdk/commit/7146104f Stats: 19 lines in 1 file changed: 13 ins; 1 del; 5 mod 8265421: java/lang/String/StringRepeat.java test is missing a memory requirement Reviewed-by: jlaskey, shade, ryadav - PR: https://git.openjdk.java.net/jdk/pull/3566
Re: RFR: 8265421: java/lang/String/StringRepeat.java test is missing a memory requirement [v2]
On Mon, 19 Apr 2021 12:50:07 GMT, Christoph Göttschkes wrote: >> Please review the following patch, which fixes the mentioned test case for >> memory constrained devices. This can be tested on a linux based development >> machine, using systemd as follows: >> >> >> $ systemd-run --user --scope -p MemoryMax=800M -p MemorySwapMax=0 >> /usr/bin/make TEST="test/jdk/java/lang/String/StringRepeat.java" run-test >> >> >> I split up the test case in a part which only executes the small repeat >> counts, and in one part which executes the huge repeat count. The second >> part is guarded by a requirement, so it is only executed if enough memory is >> available. > > Christoph Göttschkes has updated the pull request incrementally with one > additional commit since the last revision: > > Removes -Xmx2g for the first test case instance, which doesn't require a > lot of memory. Thanks a lot for the reviews. - PR: https://git.openjdk.java.net/jdk/pull/3566
