Re: RFR: 8277072: ObjectStreamClass caches keep ClassLoaders alive [v7]
On Mon, 6 Dec 2021 12:12:44 GMT, Roman Kennke wrote:
>> The caches in ObjectStreamClass basically map WeakReference to
>> SoftReference, where the ObjectStreamClass also
>> references the same Class. That means that the cache entry, and thus the
>> class and its class-loader, will not get reclaimed, unless the GC determines
>> that memory pressure is very high.
>>
>> However, this seems bogus, because that unnecessarily keeps ClassLoaders and
>> all its classes alive much longer than necessary: as soon as a ClassLoader
>> (and all its classes) become unreachable, there is no point in retaining the
>> stuff in OSC's caches.
>>
>> The proposed change is to use WeakReference instead of SoftReference for the
>> values in caches.
>>
>> Testing:
>> - [x] tier1
>> - [x] tier2
>> - [x] tier3
>> - [ ] tier4
>
> Roman Kennke has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fix cast; add testcase to cover memory pressure
Changes requested by plevart (Reviewer).
test/jdk/java/io/ObjectStreamClass/TestOSCClassLoaderLeak.java line 84:
> 82:
> 83:
> assertNotNull(ObjectStreamClass.lookup(TestClass.class).getFields());
> 84: }
I don't quite get this test. It loads ObjectStreamClass_MemoryLeakExample class
from child class loader, constructs an instance from it and calls .toString()
on an instance. This is just to indicate that the class initializer of that
class did lookup an ObjectStreamClass instance for Test class loaded by the
same child loader. OK so far...
Then there is this loop that tries to exhibit some memory pressure while
constantly looking up OSC for another Test class (this time loaded by parent
class loader) presumably to trigger clearing the SoftReference(s) of both
classes loaded by child ClassLoader Is this what the loop was supposed to
do?
And finally there is an assertNotNull that does another lookup for OSC of Test
class loaded by parent class loader, retrive its fields and check that the
returned OSC instance as well as the field array are not null. This will always
succeed regardless of what you do before the assertion.
I don't think you need any custom class loading to verify the correctness of
caching. The following two tests pass on old implementation of OSC. Do they
pass on the new one too?
public class ObjectStreamClassCaching {
@Test
public void testCachingEffectiveness() throws Exception {
var ref = lookupObjectStreamClass(TestClass.class);
System.gc();
Thread.sleep(100L);
// to trigger any ReferenceQueue processing...
lookupObjectStreamClass(AnotherTestClass.class);
Assertions.assertFalse(ref.refersTo(null),
"Cache lost entry although memory was not under
pressure");
}
@Test
public void testCacheReleaseUnderMemoryPressure() throws Exception {
var ref = lookupObjectStreamClass(TestClass.class);
pressMemoryHard(ref);
System.gc();
Thread.sleep(100L);
Assertions.assertTrue(ref.refersTo(null),
"Cache still has entry although memory was
pressed hard");
}
// separate method so that the looked-up ObjectStreamClass is not kept on
stack
private static WeakReference lookupObjectStreamClass(Class cl) {
return new WeakReference<>(ObjectStreamClass.lookup(cl));
}
private static void pressMemoryHard(Reference ref) {
try {
var list = new ArrayList<>();
while (!ref.refersTo(null)) {
list.add(new byte[1024 * 1024 * 64]); // 64 MiB chunks
}
} catch (OutOfMemoryError e) {
// release
}
}
}
class TestClass implements Serializable {
}
class AnotherTestClass implements Serializable {
}
-
PR: https://git.openjdk.java.net/jdk/pull/6375
Re: RFR: 8278311: Debian packaging doesn't work
On Mon, 6 Dec 2021 19:20:00 GMT, Alexey Semenyuk wrote: > 8278311: Debian packaging doesn't work Marked as reviewed by almatvee (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6726
Re: RFR: 8278087: Deserialization filter and filter factory property error reporting under specified [v2]
On Mon, 6 Dec 2021 16:59:41 GMT, Roger Riggs wrote: >> The effects of invalid values of `jdk.serialFilter` and >> `jdk.serialFilterFactory` properties are >> incompletely specified. The behavior for invalid values of the properties is >> different and >> use an unconventional exception type, `ExceptionInInitializerError` and >> leave the `OIF.Config` class >> uninitialized. >> >> The exceptions in the `ObjectInputFilter.Config` class initialization caused >> by invalid values of the two properties, >> either by system properties supplied on the command line or security >> properties are logged. >> The `Config` class marks either or both the filter and filter factory values >> as unusable >> and remembers the exception message. >> >> Subsequent calls to the methods that get or set the filter or filter factory >> or create >> an `ObjectInputStream` throw `java.lang.IllegalStateException` with the >> remembered exception message. >> Constructing an `ObjectInputStream` calls both `Config.getSerialFilter` and >> `Config.getSerialFilterFactory`. >> The nature of the invalid property is reported as an `IllegalStateException` >> on first use. >> >> This PR supercedes https://github.com/openjdk/jdk/pull/6508 Document that >> setting an invalid property jdk.serialFilter disables deserialization > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Address review comments to consistently identify security property names > and use the correct bug number in the test. Thank you Roger for the updates. This looks fine to me. - PR: https://git.openjdk.java.net/jdk/pull/6645
Re: Reflect arrayElementGetter
On Mon, Dec 6, 2021 at 8:10 AM Thiago Henrique Hupner wrote: > > Hi all. > > I've found something weird. > If I try to "reflect" a MethodHandle.arrayElementGetter, the method > returned is not accessible, so I need to call setAccessible to be able to > use it. > The same happens to arrayElementSetter, > > I'd like to know if this is the expected behavior. I was assuming that > it would return me some java.lang.reflect.Array, but it seems to be > returning > an internal implementation. This is expected behaviour. The MethodHandle::arrayElementGetter & ::arrayElementSetter methods produce a MethodHandle that reads / writes an array element "as if by the aaload / aastore bytecode". Those "as if" words are key as the design intent behind MethodHandles is that they operate as close to the bytecode semantics as possible. The j.l.r.Array implementations are native methods so binding to them - and, at least conceptually, wrapping them with an MH::asType operation to correct the signature - would be less performant than the bytecode based access the current implementation supports. Huge caveats apply to this reasoning. MethodHandles that aren't directly looked up using a MHs::Lookup are generally not intended to be cracked and the result may change from release to release as better implementations get explored. --Dan
Re: RFR: JDK-8273146: Start of release updates for JDK 19 [v9]
> The time to get JDK 19 underway draws nigh, please review this usual set of > start-of-release updates, including CSRs for the javac and javax.lang.model > updates: > > JDK-8277512: Add SourceVersion.RELEASE_19 > https://bugs.openjdk.java.net/browse/JDK-8277512 > > JDK-8277514: Add source 19 and target 19 to javac > https://bugs.openjdk.java.net/browse/JDK-8277514 > > Clean local tier 1 test runs for langtools, jdk, and hotspot. Joe Darcy has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 17 commits: - Merge branch 'master' into JDK-8273146 - Merge branch 'master' into JDK-8273146 - Merge branch 'master' into JDK-8273146 - Merge branch 'master' into JDK-8273146 - Update symbol files to JDK 18 b26. - Merge branch 'master' into JDK-8273146 - Merge branch 'master' into JDK-8273146 - Merge branch 'master' into JDK-8273146 - Merge branch 'master' into JDK-8273146 - Update symbol information for JDK 18 b25. - ... and 7 more: https://git.openjdk.java.net/jdk/compare/ea8d3c92...9f68398a - Changes: https://git.openjdk.java.net/jdk/pull/6493/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6493=08 Stats: 3870 lines in 67 files changed: 3827 ins; 4 del; 39 mod Patch: https://git.openjdk.java.net/jdk/pull/6493.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6493/head:pull/6493 PR: https://git.openjdk.java.net/jdk/pull/6493
Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v20]
On Mon, 29 Nov 2021 19:08:43 GMT, Lance Andersen wrote: >>> @AlanBateman yes, see above comment, thanks >> >> This is a significant change to the ZipEntry API that will require >> discussion and agreement. Can you start a discussion on core-libs-dev about >> the topic? You could start with a summary of the problem and the API and >> non-API options that have been explored. > >> > @AlanBateman yes, see above comment, thanks >> >> This is a significant change to the ZipEntry API that will require >> discussion and agreement. Can you start a discussion on core-libs-dev about >> the topic? You could start with a summary of the problem and the API and >> non-API options that have been explored. > > I agree with Alan. We are too close to RDP 1 to consider this type of change > for JDK 18 as we need more time to discuss, update the CSR, test (and we will > need additional tests for this), and give it time to bake. IMHO this will > need to go into JDK 19 assuming we move forward with changes similar to the > latest commit @LanceAndersen i've done a cleanup of the CSR, I think it looks consistent now: https://bugs.openjdk.java.net/browse/JDK-8277755 thanks - PR: https://git.openjdk.java.net/jdk/pull/6481
Re: RFR: 8275721: Name of UTC timezone in a locale changes depending on previous code [v4]
On Mon, 6 Dec 2021 20:32:55 GMT, Naoto Sato wrote: >> Fixing time zone name provider for CLDR. In some cases, COMPAT's `UTC` >> display names were incorrectly substituted for CLDR. The reason it worked >> fine after `zh-Hant-HK` was that by loading names for `zh-Hant-HK`, the >> names for `zh-Hant` were cached and hit for the following `zh-MO` name >> retrieval. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Removed unused import Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6709
Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v20]
On Mon, 6 Dec 2021 19:11:45 GMT, Andrew Leonard wrote: >> Add a new --source-date (epoch seconds) option to jar and jmod >> to allow specification of time to use for created/updated jar/jmod entries. >> This then allows the ability to make the content deterministic. >> >> Signed-off-by: Andrew Leonard > > Andrew Leonard has updated the pull request incrementally with one additional > commit since the last revision: > > 8276766: Enable jar and jmod to produce deterministic timestamped content > > Signed-off-by: Andrew Leonard The current revision looks good. The CSR needs similar clean-up for consistency as you just did for the PR. Do you want to make a pass through it? - PR: https://git.openjdk.java.net/jdk/pull/6481
Re: RFR: JDK-8276447 Deprecate finalization-related methods for removal [v4]
> Here are the code changes for the "Deprecate finalizers in the standard Java
> API" portion of JEP 421 ("Deprecate Finalization for Removal") for code
> review.
>
> This change makes the indicated deprecations, and updates the API spec for
> JEP 421. It also updates the relevant @SuppressWarning annotations.
>
> The CSR has been approved.
> An automated test build+test run passes cleanly (FWIW :D ).
Brent Christian has updated the pull request with a new target base due to a
merge or a rebase. The pull request now contains 34 commits:
- Merge branch 'master' into 8276447
- Merge branch 'master' into 8276447
- Merge branch 'master' into 8276447
- merge
- @SuppressWarnings(removal) in Windows CKey.java
- Add jls tag to runFinalization methods
- Update wording of Object.finalize, new paragraph is now bold
- update Object.finalize javadoc
- update Object.finalize JavaDoc and @deprecated tag
- Update Object.finalize deprecation message
- ... and 24 more: https://git.openjdk.java.net/jdk/compare/ea8d3c92...0a983d51
-
Changes: https://git.openjdk.java.net/jdk/pull/6465/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6465=03
Stats: 194 lines in 62 files changed: 54 ins; 38 del; 102 mod
Patch: https://git.openjdk.java.net/jdk/pull/6465.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/6465/head:pull/6465
PR: https://git.openjdk.java.net/jdk/pull/6465
Re: RFR: 8275721: Name of UTC timezone in a locale changes depending on previous code [v3]
On Mon, 6 Dec 2021 18:52:51 GMT, Joe Wang wrote:
>> Naoto Sato has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Use isFixedOffset() instead of useDaylightTime()
>
> src/java.base/share/classes/sun/util/cldr/CLDRTimeZoneNameProviderImpl.java
> line 31:
>
>> 29:
>> 30: import java.text.MessageFormat;
>> 31: import java.time.ZoneId;
>
> nit, the import is not used?
Right. Removed.
> test/jdk/sun/util/resources/TimeZone/ChineseTimeZoneNameTest.java line 66:
>
>> 64:
>> 65: @Test(dataProvider="locales")
>> 66: public void test_ChineseTimeZoneNames(Locale testLoc, Locale
>> resourceLoc) {
>
> Does this test exercise the problem as in the original test? Do we need a
> test to reproduce the calling sequence as the original test?
>
> The original issue seems to me was that the names were dependent on the order
> of calls, due to the name being cached. That is, if utcNameIn("zh-Hant-HK")
> is called first, subsequent call utcNameIn("zh-MO") would get the cached name
> from the call (zh-Hant-HK), but if the later is called first, it would not
> have the problem, that is, the following would return a correct name (協調世界時間):
> utcNameIn("zh-MO"); utcNameIn("zh-Hant-HK"); utcNameIn("zh-MO");
`協調世界時間` is actually coming from `COMPAT` provider, which is wrong. The
expected name `世界標準時間` in CLDR is in `zh-Hant` resource bundle which is not
available in `COMPAT` provider. Thus, comparing `zh-MO` and `zh-Hant` names
effectively asserts that the name is correctly coming from `CLDR` provider. I
confirmed that this regression test did fail with the runtime without the
proposed fix.
-
PR: https://git.openjdk.java.net/jdk/pull/6709
Re: RFR: 8275721: Name of UTC timezone in a locale changes depending on previous code [v4]
> Fixing time zone name provider for CLDR. In some cases, COMPAT's `UTC` > display names were incorrectly substituted for CLDR. The reason it worked > fine after `zh-Hant-HK` was that by loading names for `zh-Hant-HK`, the names > for `zh-Hant` were cached and hit for the following `zh-MO` name retrieval. Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Removed unused import - Changes: - all: https://git.openjdk.java.net/jdk/pull/6709/files - new: https://git.openjdk.java.net/jdk/pull/6709/files/cf217f56..8f96e833 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6709=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6709=02-03 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6709.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6709/head:pull/6709 PR: https://git.openjdk.java.net/jdk/pull/6709
Re: RFR: JDK-8277520: Implement JDK-8 default methods for IdentityHashMap
On 12/4/2021 4:31 PM, liach wrote: On Wed, 24 Nov 2021 05:16:40 GMT, liach wrote: Might need a CSR as now `computeIfAbsent` `computeIfPresent` `compute` `merge` would throw CME if the functions modified the map itself, and there are corresponding specification changes. Since I don't have an account on the JBS, I will post the CSR markdown draft here and hope someone can create one for me. Created CSR https://bugs.openjdk.java.net/browse/JDK-8278313. -Joe
Re: RFR: 8277863: Deprecate sun.misc.Unsafe methods that return offsets [v2]
On Mon, 6 Dec 2021 18:19:39 GMT, Alan Bateman wrote: >> Deprecate the sun.misc.Unsafe methods that return field offsets. These >> method are an impediment to possible future changes. Layout may not be fixed >> in the future, the VM should be allowed to re-layout dynamically based on >> patterns of usage. We also have the issue of libraries using these methods >> to get offsets (sometimes of classes with the same layout as JDK classes) so >> they can directly access the fields of privileged classes. It's untenable >> for libraries to rely on this going forward. >> >> The java.lang.invoke.VarHandle API (added in Java 9) provides a strongly >> typed reference to a variable that is a safe and a much better alternative >> to many cases that use these methods. Deprecating these method provides a >> gentle nudge in that directory. Once the Panama memory APIs are permanent >> then we can look at terminally deprecating and removing these methods, along >> with the accessors. > > Alan Bateman 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 three additional > commits since the last revision: > > - Add staticFieldBase to the list > - Merge > - Initial commit Marked as reviewed by psandoz (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6700
RFR: 8278311: Debian packaging doesn't work
8278311: Debian packaging doesn't work - Commit messages: - Changes from another fix removed - 8278311: Debian packaging doesn't work Changes: https://git.openjdk.java.net/jdk/pull/6726/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6726=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8278311 Stats: 33 lines in 2 files changed: 26 ins; 0 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/6726.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6726/head:pull/6726 PR: https://git.openjdk.java.net/jdk/pull/6726
Re: RFR: 8275721: Name of UTC timezone in a locale changes depending on previous code [v3]
On Sat, 4 Dec 2021 22:34:50 GMT, Naoto Sato wrote:
>> Fixing time zone name provider for CLDR. In some cases, COMPAT's `UTC`
>> display names were incorrectly substituted for CLDR. The reason it worked
>> fine after `zh-Hant-HK` was that by loading names for `zh-Hant-HK`, the
>> names for `zh-Hant` were cached and hit for the following `zh-MO` name
>> retrieval.
>
> Naoto Sato has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Use isFixedOffset() instead of useDaylightTime()
src/java.base/share/classes/sun/util/cldr/CLDRTimeZoneNameProviderImpl.java
line 31:
> 29:
> 30: import java.text.MessageFormat;
> 31: import java.time.ZoneId;
nit, the import is not used?
test/jdk/sun/util/resources/TimeZone/ChineseTimeZoneNameTest.java line 66:
> 64:
> 65: @Test(dataProvider="locales")
> 66: public void test_ChineseTimeZoneNames(Locale testLoc, Locale
> resourceLoc) {
Does this test exercise the problem as in the original test? Do we need a test
to reproduce the calling sequence as the original test?
The original issue seems to me was that the names were dependent on the order
of calls, due to the name being cached. That is, if utcNameIn("zh-Hant-HK") is
called first, subsequent call utcNameIn("zh-MO") would get the cached name from
the call (zh-Hant-HK), but if the later is called first, it would not have the
problem, that is, the following would return a correct name (協調世界時間):
utcNameIn("zh-MO"); utcNameIn("zh-Hant-HK"); utcNameIn("zh-MO");
-
PR: https://git.openjdk.java.net/jdk/pull/6709
Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v19]
On Mon, 6 Dec 2021 19:04:06 GMT, Andrew Leonard wrote: >> src/jdk.jartool/share/classes/sun/tools/jar/resources/jar.properties line >> 300: >> >>> 298: \ --date=TIMESTAMP The timestamp in ISO-8601 extended >>> offset date-time with\n\ >>> 299: \ optional time-zone format, to use for >>> the timestamps of\n\ >>> 300: \ entries, eg. "2022-02-12T12:30:00-05:00" >> >> The updated wording looks much better. Just a minor nit, it should be "e.g." >> rather than "eg.". > > ah yes, should have known that, and I did Latin at school ! fixed - PR: https://git.openjdk.java.net/jdk/pull/6481
Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v19]
On Mon, 6 Dec 2021 16:36:37 GMT, Alan Bateman wrote: >> Andrew Leonard has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8276766: Enable jar and jmod to produce deterministic timestamped content >> >> Signed-off-by: Andrew Leonard > > src/jdk.jartool/share/classes/sun/tools/jar/resources/jar.properties line 300: > >> 298: \ --date=TIMESTAMP The timestamp in ISO-8601 extended offset >> date-time with\n\ >> 299: \ optional time-zone format, to use for the >> timestamps of\n\ >> 300: \ entries, eg. "2022-02-12T12:30:00-05:00" > > The updated wording looks much better. Just a minor nit, it should be "e.g." > rather than "eg.". ah yes, should have known that, and I did Latin at school ! - PR: https://git.openjdk.java.net/jdk/pull/6481
Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v20]
> Add a new --source-date (epoch seconds) option to jar and jmod to > allow specification of time to use for created/updated jar/jmod entries. This > then allows the ability to make the content deterministic. > > Signed-off-by: Andrew Leonard Andrew Leonard has updated the pull request incrementally with one additional commit since the last revision: 8276766: Enable jar and jmod to produce deterministic timestamped content Signed-off-by: Andrew Leonard - Changes: - all: https://git.openjdk.java.net/jdk/pull/6481/files - new: https://git.openjdk.java.net/jdk/pull/6481/files/c5104305..59617359 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6481=19 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6481=18-19 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/6481.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6481/head:pull/6481 PR: https://git.openjdk.java.net/jdk/pull/6481
Re: RFR: 8277863: Deprecate sun.misc.Unsafe methods that return offsets [v2]
On Mon, 6 Dec 2021 18:19:39 GMT, Alan Bateman wrote: >> Deprecate the sun.misc.Unsafe methods that return field offsets. These >> method are an impediment to possible future changes. Layout may not be fixed >> in the future, the VM should be allowed to re-layout dynamically based on >> patterns of usage. We also have the issue of libraries using these methods >> to get offsets (sometimes of classes with the same layout as JDK classes) so >> they can directly access the fields of privileged classes. It's untenable >> for libraries to rely on this going forward. >> >> The java.lang.invoke.VarHandle API (added in Java 9) provides a strongly >> typed reference to a variable that is a safe and a much better alternative >> to many cases that use these methods. Deprecating these method provides a >> gentle nudge in that directory. Once the Panama memory APIs are permanent >> then we can look at terminally deprecating and removing these methods, along >> with the accessors. > > Alan Bateman 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 three additional > commits since the last revision: > > - Add staticFieldBase to the list > - Merge > - Initial commit It's okay to leave `INVALID_FIELD_OFFSET` as is as it's also for `arrayBaseOffset`. - Marked as reviewed by mchung (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6700
Re: RFR: 8277863: Deprecate sun.misc.Unsafe methods that return offsets [v2]
> Deprecate the sun.misc.Unsafe methods that return field offsets. These method > are an impediment to possible future changes. Layout may not be fixed in the > future, the VM should be allowed to re-layout dynamically based on patterns > of usage. We also have the issue of libraries using these methods to get > offsets (sometimes of classes with the same layout as JDK classes) so they > can directly access the fields of privileged classes. It's untenable for > libraries to rely on this going forward. > > The java.lang.invoke.VarHandle API (added in Java 9) provides a strongly > typed reference to a variable that is a safe and a much better alternative to > many cases that use these methods. Deprecating these method provides a gentle > nudge in that directory. Once the Panama memory APIs are permanent then we > can look at terminally deprecating and removing these methods, along with the > accessors. Alan Bateman 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 three additional commits since the last revision: - Add staticFieldBase to the list - Merge - Initial commit - Changes: - all: https://git.openjdk.java.net/jdk/pull/6700/files - new: https://git.openjdk.java.net/jdk/pull/6700/files/794c6160..40bb5e7e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6700=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6700=00-01 Stats: 5387 lines in 366 files changed: 3786 ins; 549 del; 1052 mod Patch: https://git.openjdk.java.net/jdk/pull/6700.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6700/head:pull/6700 PR: https://git.openjdk.java.net/jdk/pull/6700
Re: RFR: 8277863: Deprecate sun.misc.Unsafe methods that return offsets
On Mon, 6 Dec 2021 17:57:49 GMT, Paul Sandoz wrote: > Yes, I think you should include it, it's part of this set of functionality > (since base is used in conjunction with offsets) Okay, done. - PR: https://git.openjdk.java.net/jdk/pull/6700
Re: RFR: 8003417: WeakHashMap$HashIterator removes wrong entry
On Sat, 20 Nov 2021 10:08:41 GMT, Jaikiran Pai wrote: > Can I please get a review for this change which proposes to fix the issue > reported in https://bugs.openjdk.java.net/browse/JDK-8003417? > > The issue notes that this is applicable for `WeakHashMap` which have `null` > keys. However, the issue is even applicable for `WeakHashMap` instances which > don't have `null` keys, as reproduced and shown by the newly added jtreg test > case in this PR. > > The root cause of the issue is that once the iterator is used to iterate till > the end and the `remove()` is called, then the > `WeakHashMap$HashIterator#remove()` implementation used to pass `null` as the > key to remove from the map, instead of the key of the last returned entry. > The commit in this PR fixes that part. > > A new jtreg test has been added which reproduces the issue as well as > verifies the fix. > `tier1` testing and this new test have passed after this change. However, I > guess this will require a JCK run to be run too, perhaps? If so, I will need > help from someone who has access to them to have this run against those > please. The value of `currentKey` should track the value of `lastReturned.get()` and does so in the `nextEntry()` and `remove()` methods. The odd statement is in `hasNext` that set the state of `currentkey` to null independently of `lastReturned`. Leaving it non-null will allow `remove` to operate correctly but may have a downside of keeping a strong reference to last key until the HashIterator is GC'd. The `remove()` method can correctly use `currentKey` to remove the last entry returned. - Changes requested by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6488
Re: RFR: 8277863: Deprecate sun.misc.Unsafe methods that return offsets
On Mon, 6 Dec 2021 17:47:37 GMT, Alan Bateman wrote: > I toyed with including staticFieldBase but I didn't find anything like the > usage as the offset methods. Easy to include if you think it's worth doing. Yes, I think you should include it, it's part of this set of functionality (since base is used in conjunction with offsets) - PR: https://git.openjdk.java.net/jdk/pull/6700
Re: RFR: JDK-8276447 Deprecate finalization-related methods for removal [v3]
On Wed, 1 Dec 2021 19:23:59 GMT, Brent Christian wrote:
>> Here are the code changes for the "Deprecate finalizers in the standard Java
>> API" portion of JEP 421 ("Deprecate Finalization for Removal") for code
>> review.
>>
>> This change makes the indicated deprecations, and updates the API spec for
>> JEP 421. It also updates the relevant @SuppressWarning annotations.
>>
>> The CSR has been approved.
>> An automated test build+test run passes cleanly (FWIW :D ).
>
> Brent Christian has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains 33 commits:
>
> - Merge branch 'master' into 8276447
> - Merge branch 'master' into 8276447
> - merge
> - @SuppressWarnings(removal) in Windows CKey.java
> - Add jls tag to runFinalization methods
> - Update wording of Object.finalize, new paragraph is now bold
> - update Object.finalize javadoc
> - update Object.finalize JavaDoc and @deprecated tag
> - Update Object.finalize deprecation message
> - Indicate that runFinalizers does nothing if finalization is disabled or
> removed
> - ... and 23 more:
> https://git.openjdk.java.net/jdk/compare/0dfb3a70...8cde0674
Marked as reviewed by prr (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/6465
Re: RFR: 8277868: Use Comparable.compare() instead of surrogate code [v2]
On Mon, 29 Nov 2021 08:18:47 GMT, Сергей Цыпанов wrote:
>> Instead of something like
>>
>> long x;
>> long y;
>> return (x < y) ? -1 : ((x == y) ? 0 : 1);
>>
>> we can use `return Long.compare(x, y);`
>>
>> All replacements are done with IDE.
>
> Сергей Цыпанов has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8277868: Use Integer.signum() in BasicTableUI
src/java.desktop/share/classes/java/awt/geom/Line2D.java line 115:
> 113: */
> 114: public double getX1() {
> 115: return x1;
What do these changes have to do with the subject of the PR ?
src/java.desktop/share/classes/sun/java2d/Spans.java line 322:
> 320: float otherStart = otherSpan.getStart();
> 321:
> 322: return Float.compare(mStart, otherStart);
We don't need the variable any more, do we ?
-
PR: https://git.openjdk.java.net/jdk/pull/6575
Re: RFR: 8277863: Deprecate sun.misc.Unsafe methods that return offsets
On Fri, 3 Dec 2021 13:05:44 GMT, Alan Bateman wrote: > Deprecate the sun.misc.Unsafe methods that return field offsets. These method > are an impediment to possible future changes. Layout may not be fixed in the > future, the VM should be allowed to re-layout dynamically based on patterns > of usage. We also have the issue of libraries using these methods to get > offsets (sometimes of classes with the same layout as JDK classes) so they > can directly access the fields of privileged classes. It's untenable for > libraries to rely on this going forward. > > The java.lang.invoke.VarHandle API (added in Java 9) provides a strongly > typed reference to a variable that is a safe and a much better alternative to > many cases that use these methods. Deprecating these method provides a gentle > nudge in that directory. Once the Panama memory APIs are permanent then we > can look at terminally deprecating and removing these methods, along with the > accessors. Yes, we have to keep chipping away at this. Most of the on-heap usages that I found could have been replaced with VarHandles a long time ago but many of these projects are still building to old JDK releases and maybe MR JARs are still problematic in the build. The great progress on the memory API in Project Panama means it won't be long before those accessors are needed for off-heap. So yes, a few more releases and we should be able to make more progress. I toyed with including staticFieldBase but I didn't find anything like the usage as the offset methods. Easy to include if you think it's worth doing. - PR: https://git.openjdk.java.net/jdk/pull/6700
Re: RFR: 8278185: Custom JRE cannot find non-ASCII named module inside
On Fri, 3 Dec 2021 07:29:17 GMT, Toshio Nakamura wrote: > Could you review this fix? > > Problem: > Custom JRE generated by jlink cannot find non-ASCII named modules included > inside the JRE. > > Cause and fix: > If module or package name was composed by ASCII then non-ASCII characters, > ImageStringsReader:stringFromByteBufferMatches() miscalculated the length of > matched string. The first part of ASCII characters was missing. This patch > corrected the value. > > Testing: > tier1 and tier2 on Linux have no regression. > I wasn't able to create an automate test for this issue. I appreciate any > advice. Regression tests for `ResourceBundle` class have tests that dynamically generate modules. Although I have not used non-ASCII module names, tests under `test/jdk/java/util/ResourceBundle/modules` may shed some light on the subject. - PR: https://git.openjdk.java.net/jdk/pull/6693
Integrated: JDK-8278273: Remove unnecessary exclusion of doclint accessibility checks
On Sun, 5 Dec 2021 23:45:32 GMT, Joe Darcy wrote: > Exploratory builds indicate it is not currently necessary to exclude the > doclint accessibility checks; this patch enables them. > > (Enabling the reference checks is left for future work.) This pull request has now been integrated. Changeset: 5045eb53 Author:Joe Darcy URL: https://git.openjdk.java.net/jdk/commit/5045eb538b3afc6cf646642f1109473597b3004a Stats: 14 lines in 7 files changed: 0 ins; 0 del; 14 mod 8278273: Remove unnecessary exclusion of doclint accessibility checks Reviewed-by: iris, alanb, ihse - PR: https://git.openjdk.java.net/jdk/pull/6713
Re: RFR: 8278087: Deserialization filter and filter factory property error reporting under specified [v2]
> The effects of invalid values of `jdk.serialFilter` and > `jdk.serialFilterFactory` properties are > incompletely specified. The behavior for invalid values of the properties is > different and > use an unconventional exception type, `ExceptionInInitializerError` and leave > the `OIF.Config` class > uninitialized. > > The exceptions in the `ObjectInputFilter.Config` class initialization caused > by invalid values of the two properties, > either by system properties supplied on the command line or security > properties are logged. > The `Config` class marks either or both the filter and filter factory values > as unusable > and remembers the exception message. > > Subsequent calls to the methods that get or set the filter or filter factory > or create > an `ObjectInputStream` throw `java.lang.IllegalStateException` with the > remembered exception message. > Constructing an `ObjectInputStream` calls both `Config.getSerialFilter` and > `Config.getSerialFilterFactory`. > The nature of the invalid property is reported as an `IllegalStateException` > on first use. > > This PR supercedes https://github.com/openjdk/jdk/pull/6508 Document that > setting an invalid property jdk.serialFilter disables deserialization Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Address review comments to consistently identify security property names and use the correct bug number in the test. - Changes: - all: https://git.openjdk.java.net/jdk/pull/6645/files - new: https://git.openjdk.java.net/jdk/pull/6645/files/4dec7f48..52ab7b5b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6645=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6645=00-01 Stats: 13 lines in 2 files changed: 3 ins; 4 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/6645.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6645/head:pull/6645 PR: https://git.openjdk.java.net/jdk/pull/6645
Re: RFR: 8277863: Deprecate sun.misc.Unsafe methods that return offsets
On Fri, 3 Dec 2021 13:05:44 GMT, Alan Bateman wrote: > Deprecate the sun.misc.Unsafe methods that return field offsets. These method > are an impediment to possible future changes. Layout may not be fixed in the > future, the VM should be allowed to re-layout dynamically based on patterns > of usage. We also have the issue of libraries using these methods to get > offsets (sometimes of classes with the same layout as JDK classes) so they > can directly access the fields of privileged classes. It's untenable for > libraries to rely on this going forward. > > The java.lang.invoke.VarHandle API (added in Java 9) provides a strongly > typed reference to a variable that is a safe and a much better alternative to > many cases that use these methods. Deprecating these method provides a gentle > nudge in that directory. Once the Panama memory APIs are permanent then we > can look at terminally deprecating and removing these methods, along with the > accessors. Slowly chipping away... but we are getting really close to being able to deprecate all the unsafe accessor methods. Did you also consider the method `staticFieldBase` and the field `INVALID_FIELD_OFFSET`? I think those could also be included since the aim is to discourage unsafe access to fields of objects. - PR: https://git.openjdk.java.net/jdk/pull/6700
Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v19]
On Mon, 6 Dec 2021 13:57:48 GMT, Andrew Leonard wrote: >> Add a new --source-date (epoch seconds) option to jar and jmod >> to allow specification of time to use for created/updated jar/jmod entries. >> This then allows the ability to make the content deterministic. >> >> Signed-off-by: Andrew Leonard > > Andrew Leonard has updated the pull request incrementally with one additional > commit since the last revision: > > 8276766: Enable jar and jmod to produce deterministic timestamped content > > Signed-off-by: Andrew Leonard src/jdk.jartool/share/classes/sun/tools/jar/resources/jar.properties line 300: > 298: \ --date=TIMESTAMP The timestamp in ISO-8601 extended offset > date-time with\n\ > 299: \ optional time-zone format, to use for the > timestamps of\n\ > 300: \ entries, eg. "2022-02-12T12:30:00-05:00" The updated wording looks much better. Just a minor nit, it should be "e.g." rather than "eg.". - PR: https://git.openjdk.java.net/jdk/pull/6481
RFR: 8277863: Deprecate sun.misc.Unsafe methods that return offsets
Deprecate the sun.misc.Unsafe methods that return field offsets. These method are an impediment to possible future changes. Layout may not be fixed in the future, the VM should be allowed to re-layout dynamically based on patterns of usage. We also have the issue of libraries using these methods to get offsets (sometimes of classes with the same layout as JDK classes) so they can directly access the fields of privileged classes. It's untenable for libraries to rely on this going forward. The java.lang.invoke.VarHandle API (added in Java 9) provides a strongly typed reference to a variable that is a safe and a much better alternative to many cases that use these methods. Deprecating these method provides a gentle nudge in that directory. Once the Panama memory APIs are permanent then we can look at terminally deprecating and removing these methods, along with the accessors. - Commit messages: - Initial commit Changes: https://git.openjdk.java.net/jdk/pull/6700/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6700=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8277863 Stats: 14 lines in 1 file changed: 14 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6700.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6700/head:pull/6700 PR: https://git.openjdk.java.net/jdk/pull/6700
Re: RFR: 8278087: Deserialization filter and filter factory property error reporting under specified
On Mon, 6 Dec 2021 04:44:18 GMT, Jaikiran Pai wrote: >> The effects of invalid values of `jdk.serialFilter` and >> `jdk.serialFilterFactory` properties are >> incompletely specified. The behavior for invalid values of the properties is >> different and >> use an unconventional exception type, `ExceptionInInitializerError` and >> leave the `OIF.Config` class >> uninitialized. >> >> The exceptions in the `ObjectInputFilter.Config` class initialization caused >> by invalid values of the two properties, >> either by system properties supplied on the command line or security >> properties are logged. >> The `Config` class marks either or both the filter and filter factory values >> as unusable >> and remembers the exception message. >> >> Subsequent calls to the methods that get or set the filter or filter factory >> or create >> an `ObjectInputStream` throw `java.lang.IllegalStateException` with the >> remembered exception message. >> Constructing an `ObjectInputStream` calls both `Config.getSerialFilter` and >> `Config.getSerialFilterFactory`. >> The nature of the invalid property is reported as an `IllegalStateException` >> on first use. >> >> This PR supercedes https://github.com/openjdk/jdk/pull/6508 Document that >> setting an invalid property jdk.serialFilter disables deserialization > > src/java.base/share/classes/java/io/ObjectInputFilter.java line 859: > >> 857: /* >> 858: * Return message for an invalid filter factory configuration >> saved from the static init. >> 859: * It can be called before the static initializer is complete >> and has set the message/null. > > More of a curiosity than a review comment - Is that because the > `Config.getSerialFilterFactory()/setSerialFilterFactory(...)` could be called > from the constructor of the user configured factory class, which gets invoked > when static initialization is in progress for the `Config` class? Yes, it can be called before the `Config` class has completed initialization and `invalidFactoryMessage` may be seen as null. - PR: https://git.openjdk.java.net/jdk/pull/6645
Re: RFR: 8278087: Deserialization filter and filter factory property error reporting under specified
On Mon, 6 Dec 2021 04:40:01 GMT, Jaikiran Pai wrote:
>> The effects of invalid values of `jdk.serialFilter` and
>> `jdk.serialFilterFactory` properties are
>> incompletely specified. The behavior for invalid values of the properties is
>> different and
>> use an unconventional exception type, `ExceptionInInitializerError` and
>> leave the `OIF.Config` class
>> uninitialized.
>>
>> The exceptions in the `ObjectInputFilter.Config` class initialization caused
>> by invalid values of the two properties,
>> either by system properties supplied on the command line or security
>> properties are logged.
>> The `Config` class marks either or both the filter and filter factory values
>> as unusable
>> and remembers the exception message.
>>
>> Subsequent calls to the methods that get or set the filter or filter factory
>> or create
>> an `ObjectInputStream` throw `java.lang.IllegalStateException` with the
>> remembered exception message.
>> Constructing an `ObjectInputStream` calls both `Config.getSerialFilter` and
>> `Config.getSerialFilterFactory`.
>> The nature of the invalid property is reported as an `IllegalStateException`
>> on first use.
>>
>> This PR supercedes https://github.com/openjdk/jdk/pull/6508 Document that
>> setting an invalid property jdk.serialFilter disables deserialization
>
> src/java.base/share/classes/java/io/ObjectInputFilter.java line 751:
>
>> 749: if (serialFilter != null) {
>> 750: throw new IllegalStateException("Serial filter can
>> only be set once");
>> 751: } else if (invalidFilterMessage != null) {
>
> The `invalidFilterMessage` is a `static` `final` `String` which gets
> initialized during the static initialization of the class and as such doesn't
> get changed after that. Do you think this lock on `serialFilterLock` is
> necessary for this check or it can be moved outside this `synchronized` block?
Checking `serialFilter` for non-null required the synchronization.
The check of `invalidFilterMessage` can be pulled out of the synchronization
since in expected usage the method is only called once so the performance isn't
significant.
-
PR: https://git.openjdk.java.net/jdk/pull/6645
Re: RFR: 8278087: Deserialization filter and filter factory property error reporting under specified
On Mon, 6 Dec 2021 04:30:16 GMT, Jaikiran Pai wrote:
>> The effects of invalid values of `jdk.serialFilter` and
>> `jdk.serialFilterFactory` properties are
>> incompletely specified. The behavior for invalid values of the properties is
>> different and
>> use an unconventional exception type, `ExceptionInInitializerError` and
>> leave the `OIF.Config` class
>> uninitialized.
>>
>> The exceptions in the `ObjectInputFilter.Config` class initialization caused
>> by invalid values of the two properties,
>> either by system properties supplied on the command line or security
>> properties are logged.
>> The `Config` class marks either or both the filter and filter factory values
>> as unusable
>> and remembers the exception message.
>>
>> Subsequent calls to the methods that get or set the filter or filter factory
>> or create
>> an `ObjectInputStream` throw `java.lang.IllegalStateException` with the
>> remembered exception message.
>> Constructing an `ObjectInputStream` calls both `Config.getSerialFilter` and
>> `Config.getSerialFilterFactory`.
>> The nature of the invalid property is reported as an `IllegalStateException`
>> on first use.
>>
>> This PR supercedes https://github.com/openjdk/jdk/pull/6508 Document that
>> setting an invalid property jdk.serialFilter disables deserialization
>
> src/java.base/share/classes/java/io/ObjectInputFilter.java line 532:
>
>> 530: * invalid serial filter.
>> 531: * If the system property {@code jdk.serialFilter} or the {@link
>> java.security.Security}
>> 532: * property is not set the filter can be set with
>
>> or the {@link java.security.Security} property is not set the filter can be
>> set ...
>
> Is it intentional that the name of security property is left out here?
> Perhaps this should be:
> `or the {@link java.security.Security} property {@code jdk.serialFilter} is
> not set the filter `
>
> or even:
>
> `or the {@link java.security.Security} property of the same name is not set
> the filter`
Yes, for consistency, the first suggestion.
-
PR: https://git.openjdk.java.net/jdk/pull/6645
Re: RFR: 8278087: Deserialization filter and filter factory property error reporting under specified
On Mon, 6 Dec 2021 04:27:30 GMT, Jaikiran Pai wrote:
>> The effects of invalid values of `jdk.serialFilter` and
>> `jdk.serialFilterFactory` properties are
>> incompletely specified. The behavior for invalid values of the properties is
>> different and
>> use an unconventional exception type, `ExceptionInInitializerError` and
>> leave the `OIF.Config` class
>> uninitialized.
>>
>> The exceptions in the `ObjectInputFilter.Config` class initialization caused
>> by invalid values of the two properties,
>> either by system properties supplied on the command line or security
>> properties are logged.
>> The `Config` class marks either or both the filter and filter factory values
>> as unusable
>> and remembers the exception message.
>>
>> Subsequent calls to the methods that get or set the filter or filter factory
>> or create
>> an `ObjectInputStream` throw `java.lang.IllegalStateException` with the
>> remembered exception message.
>> Constructing an `ObjectInputStream` calls both `Config.getSerialFilter` and
>> `Config.getSerialFilterFactory`.
>> The nature of the invalid property is reported as an `IllegalStateException`
>> on first use.
>>
>> This PR supercedes https://github.com/openjdk/jdk/pull/6508 Document that
>> setting an invalid property jdk.serialFilter disables deserialization
>
> src/java.base/share/classes/java/io/ObjectInputFilter.java line 527:
>
>> 525: * The filter is created as if {@link #createFilter(String)
>> createFilter} is called,
>> 526: * if the filter string is invalid the initialization fails and
>> subsequent attempts to
>> 527: * {@linkplain Config#getSerialFilter() get the filter}, {@link
>> Config#setSerialFilter set a filter},
>
>> {@link Config#setSerialFilter set a filter}
>
> Should this instead be "{@link Config#setSerialFilter(ObjectInputFilter) set
> a filter}" i.e. include the param as part of the `@link`, like in other
> places?
I intended to use @linkplain, using prose with a link is more readable then a
sentence broken up with full method references.
-
PR: https://git.openjdk.java.net/jdk/pull/6645
Re: RFR: 8278044: ObjectInputStream methods invoking the OIF.CFG.getSerialFilterFactory() silent about error cases. [v2]
> The specification of ObjectInputStream constructors that invoke
> `ObjectInputFilter.Config.getSerialFilterFactory()` do not mention exceptions
> that may be thrown by the apply() method.
>
> In both constructors, add the following to the paragraph the describes
> invoking the factory:
>
> * When the filter factory {@code apply} method is invoked it may throw a
> runtime exception
> * preventing the {@code ObjectInputStream} from being constructed.
Roger Riggs has updated the pull request incrementally with one additional
commit since the last revision:
Clarified that a runtime exception may be thrown by the filter factory
returned from `ObjectInputFilter.Config.getSerialFilterFactory()`
not by the `getSerialFilterFactory` method.
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/6704/files
- new: https://git.openjdk.java.net/jdk/pull/6704/files/ecb9aa90..da95d900
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=6704=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk=6704=00-01
Stats: 4 lines in 1 file changed: 2 ins; 0 del; 2 mod
Patch: https://git.openjdk.java.net/jdk/pull/6704.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/6704/head:pull/6704
PR: https://git.openjdk.java.net/jdk/pull/6704
Re: RFR: 8277072: ObjectStreamClass caches keep ClassLoaders alive [v7]
On Mon, 6 Dec 2021 12:12:44 GMT, Roman Kennke wrote: >> The caches in ObjectStreamClass basically map WeakReference to >> SoftReference, where the ObjectStreamClass also >> references the same Class. That means that the cache entry, and thus the >> class and its class-loader, will not get reclaimed, unless the GC determines >> that memory pressure is very high. >> >> However, this seems bogus, because that unnecessarily keeps ClassLoaders and >> all its classes alive much longer than necessary: as soon as a ClassLoader >> (and all its classes) become unreachable, there is no point in retaining the >> stuff in OSC's caches. >> >> The proposed change is to use WeakReference instead of SoftReference for the >> values in caches. >> >> Testing: >> - [x] tier1 >> - [x] tier2 >> - [x] tier3 >> - [ ] tier4 > > Roman Kennke has updated the pull request incrementally with one additional > commit since the last revision: > > Fix cast; add testcase to cover memory pressure Thanks for the updates. LGTM - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6375
Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v19]
> Add a new --source-date (epoch seconds) option to jar and jmod to > allow specification of time to use for created/updated jar/jmod entries. This > then allows the ability to make the content deterministic. > > Signed-off-by: Andrew Leonard Andrew Leonard has updated the pull request incrementally with one additional commit since the last revision: 8276766: Enable jar and jmod to produce deterministic timestamped content Signed-off-by: Andrew Leonard - Changes: - all: https://git.openjdk.java.net/jdk/pull/6481/files - new: https://git.openjdk.java.net/jdk/pull/6481/files/2a2d781b..c5104305 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6481=18 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6481=17-18 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/6481.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6481/head:pull/6481 PR: https://git.openjdk.java.net/jdk/pull/6481
Re: RFR: 8277072: ObjectStreamClass caches keep ClassLoaders alive [v6]
On Sat, 4 Dec 2021 08:47:03 GMT, Peter Levart wrote:
>> Roman Kennke has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Remove unnecessary import
>
> src/java.base/share/classes/java/io/ClassCache.java line 63:
>
>> 61: protected SoftReference computeValue(Class type) {
>> 62: return new
>> SoftReference<>(ClassCache.this.computeValue(type), queue);
>> 63: }
>
> How does this work? You create a bare SoftReference here and register it with
> queue
>
> ...then down in processQueue() you pick it up and cast to CacheRef... Doesn't
> this throw ClassCastException ?
>
Indeed, good catch! Apparently no test (existing or new) covers the
memory-pressure scenario. My new test only covers the scenario when the Class
disappears altogether. I added a test to verify behaviour under memory
pressure, and pushed a fix.
-
PR: https://git.openjdk.java.net/jdk/pull/6375
Re: RFR: 8277072: ObjectStreamClass caches keep ClassLoaders alive [v7]
> The caches in ObjectStreamClass basically map WeakReference to > SoftReference, where the ObjectStreamClass also references > the same Class. That means that the cache entry, and thus the class and its > class-loader, will not get reclaimed, unless the GC determines that memory > pressure is very high. > > However, this seems bogus, because that unnecessarily keeps ClassLoaders and > all its classes alive much longer than necessary: as soon as a ClassLoader > (and all its classes) become unreachable, there is no point in retaining the > stuff in OSC's caches. > > The proposed change is to use WeakReference instead of SoftReference for the > values in caches. > > Testing: > - [x] tier1 > - [x] tier2 > - [x] tier3 > - [ ] tier4 Roman Kennke has updated the pull request incrementally with one additional commit since the last revision: Fix cast; add testcase to cover memory pressure - Changes: - all: https://git.openjdk.java.net/jdk/pull/6375/files - new: https://git.openjdk.java.net/jdk/pull/6375/files/57428e0f..e6e34cea Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6375=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6375=05-06 Stats: 22 lines in 2 files changed: 18 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/6375.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6375/head:pull/6375 PR: https://git.openjdk.java.net/jdk/pull/6375
Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v17]
On Mon, 6 Dec 2021 00:15:13 GMT, Lance Andersen wrote:
>> Andrew Leonard has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains 25 commits:
>>
>> - Merge jdk:master
>>
>>Signed-off-by: Andrew Leonard
>> - Merge branch 'master' of https://github.com/openjdk/jdk into
>> jarjmodtimestamps
>> - 8276766: Enable jar and jmod to produce deterministic timestamped content
>>
>>Signed-off-by: Andrew Leonard
>> - Merge branch 'jarjmodtimestamps' of github.com:andrew-m-leonard/jdk into
>> jarjmodtimestamps
>> - Update src/jdk.jlink/share/classes/jdk/tools/jmod/JmodOutputStream.java
>>
>>Co-authored-by: Magnus Ihse Bursie
>> - Merge branch 'master' of https://github.com/openjdk/jdk into
>> jarjmodtimestamps
>> - 8276766: Enable jar and jmod to produce deterministic timestamped content
>>
>>Signed-off-by: Andrew Leonard
>> - 8276766: Enable jar and jmod to produce deterministic timestamped content
>>
>>Signed-off-by: Andrew Leonard
>> - 8276766: Enable jar and jmod to produce deterministic timestamped content
>>
>>Signed-off-by: Andrew Leonard
>> - 8276766: Enable jar and jmod to produce deterministic timestamped content
>>
>>Signed-off-by: Andrew Leonard
>> - ... and 15 more:
>> https://git.openjdk.java.net/jdk/compare/45da3aea...06863697
>
> src/jdk.jartool/share/classes/sun/tools/jar/resources/jar.properties line 86:
>
>> 84: unexpected versioned entry {0} for release {1}
>> 85: error.date.notvalid=\
>> 86: date {0} is not a valid ISO 8601 date and time
>
> Please rephrase to use wording similar to what is in the Javadoc date-time
> with offset and zone
reworded
> src/jdk.jartool/share/classes/sun/tools/jar/resources/jar.properties line 88:
>
>> 86: date {0} is not a valid ISO 8601 date and time
>> 87: error.date.out.of.range=\
>> 88: date {0} is not within the valid range
>
> Please include the range in the message
added range
> src/jdk.jartool/share/classes/sun/tools/jar/resources/jar.properties line 298:
>
>> 296: \ -0, --no-compress Store only; use no ZIP compression
>> 297: main.help.opt.create.update.index.date=\
>> 298: \ --date=TIMESTAMP The timestamp in ISO 8601 format to use\n\
>
> Same comment as above perhaps show the syntax
done
> src/jdk.jlink/share/classes/jdk/tools/jmod/resources/jmod.properties line 111:
>
>> 109: err.no.moduleToHash=No hashes recorded: no module matching {0} found to
>> record hashes
>> 110: err.invalid.date=--date {0} is not a valid ISO 8601 date and time: {1}
>> 111: err.date.out.of.range=--date {0} is out of the valid range
>
> Same comments as above
done
> test/jdk/tools/jar/JarEntryTime.java line 191:
>
>> 189: "2038-11-26T06:06:06+00:00",
>> 190: "2098-02-18T00:00:00-08:00",
>> 191: "2099-12-31T23:59:59+00:00"};
>
> I believe the parsing format you are using supports formats such as
> '2011-12-03T10:15:30', '2011-12-03T10:15:30+01:00'
> and'2011-12-03T10:15:30+01:00[Europe/Paris]'.
>
> please add a few extra values to the above
added more tests
note '2011-12-03T10:15:30' is not valid as it needs a offset or Z, so it
represents a reproducible "instant"
-
PR: https://git.openjdk.java.net/jdk/pull/6481
Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v17]
On Fri, 3 Dec 2021 10:05:43 GMT, Andrew Leonard wrote: >> Add a new --source-date (epoch seconds) option to jar and jmod >> to allow specification of time to use for created/updated jar/jmod entries. >> This then allows the ability to make the content deterministic. >> >> Signed-off-by: Andrew Leonard > > Andrew Leonard has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 25 commits: > > - Merge jdk:master > >Signed-off-by: Andrew Leonard > - Merge branch 'master' of https://github.com/openjdk/jdk into > jarjmodtimestamps > - 8276766: Enable jar and jmod to produce deterministic timestamped content > >Signed-off-by: Andrew Leonard > - Merge branch 'jarjmodtimestamps' of github.com:andrew-m-leonard/jdk into > jarjmodtimestamps > - Update src/jdk.jlink/share/classes/jdk/tools/jmod/JmodOutputStream.java > >Co-authored-by: Magnus Ihse Bursie > - Merge branch 'master' of https://github.com/openjdk/jdk into > jarjmodtimestamps > - 8276766: Enable jar and jmod to produce deterministic timestamped content > >Signed-off-by: Andrew Leonard > - 8276766: Enable jar and jmod to produce deterministic timestamped content > >Signed-off-by: Andrew Leonard > - 8276766: Enable jar and jmod to produce deterministic timestamped content > >Signed-off-by: Andrew Leonard > - 8276766: Enable jar and jmod to produce deterministic timestamped content > >Signed-off-by: Andrew Leonard > - ... and 15 more: > https://git.openjdk.java.net/jdk/compare/45da3aea...06863697 > I looked at the latest patch > ([0686369](https://github.com/openjdk/jdk/commit/068636971e833cb582790667171a67e40c823bb1)) > and I think you've got it to a good place (and thank you to John Neffenger > for all the help). > > One comment on the usage message is that it might be a bit clearer to say > "for the timestamps of entries" rather than "for entry timestamps". I'm also > wondering if it would be useful to include an example of an ISO 8601 in the > usage message. @AlanBateman thanks. I've reworded, and added an example - PR: https://git.openjdk.java.net/jdk/pull/6481
Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v18]
> Add a new --source-date (epoch seconds) option to jar and jmod to > allow specification of time to use for created/updated jar/jmod entries. This > then allows the ability to make the content deterministic. > > Signed-off-by: Andrew Leonard Andrew Leonard has updated the pull request incrementally with one additional commit since the last revision: 8276766: Enable jar and jmod to produce deterministic timestamped content Signed-off-by: Andrew Leonard - Changes: - all: https://git.openjdk.java.net/jdk/pull/6481/files - new: https://git.openjdk.java.net/jdk/pull/6481/files/06863697..2a2d781b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6481=17 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6481=16-17 Stats: 18 lines in 3 files changed: 9 ins; 0 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/6481.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6481/head:pull/6481 PR: https://git.openjdk.java.net/jdk/pull/6481
Re: RFR: 8277992: Add fast jdk_svc subtests to jdk:tier3
On Tue, 30 Nov 2021 18:48:15 GMT, Aleksey Shipilev wrote: > OpenJDK tiered tests definitions have the catch-all `tier4` that runs all > tests not defined in the lower tiers. `hotspot:tier4` has lots of them, > mostly long-running vmTestbase tests, which take many hours even on a very > parallel machines. > > This, unfortunately, has a chilling effect on `jdk:tier4`, which is seldom > run by contributors, because `hotspot:tier4` is in the way. But, there are > plenty of fast and stable tests in `jdk:tier4` that can be run in > `jdk:tier3`. `jdk_svc` is the example of such tests: management features > (including but not limited to JFR) are important to keep from regressions, > and significant subset of them runs pretty fast. > > So, it makes sense to move some `jdk_svc` tests to `jdk:tier3` to expose it > to more contributors. I think the only group we don't want to run is > `svc_tools`, which includes lots of non-parallel tests that are rather slow. > > Sample run before: > > > == > Test summary > == >TEST TOTAL PASS FAIL ERROR > >jtreg:test/jdk:tier3174 174 0 0 > > == > TEST SUCCESS > > Finished building target 'run-test' in configuration > 'linux-x86_64-server-fastdebug' > > real 2m38.242s > user 80m7.216s > sys 2m13.846s > > > == > Test summary > == >TEST TOTAL PASS FAIL ERROR > >>> jtreg:test/jdk:tier4 2904 2901 3 0 << > == > TEST FAILURE > > real 18m13.933s > user 546m50.556s > sys 25m7.086s > > > Sample run after: > > > == > Test summary > == >TEST TOTAL PASS FAIL ERROR > >jtreg:test/jdk:tier3 1296 1296 0 0 > > == > TEST SUCCESS > > Finished building target 'run-test' in configuration > 'linux-x86_64-server-fastdebug' > > real 7m49.017s > user 287m30.943s > sys 13m20.060s > > == > Test summary > == >TEST TOTAL PASS FAIL ERROR > >>> jtreg:test/jdk:tier4 1783 1780 3 0 << > == > TEST FAILURE > > > real 12m19.973s > user 351m48.561s > sys 14m51.566s Ok, good. I need some formal (R)eviews to get this going :) - PR: https://git.openjdk.java.net/jdk/pull/6619
Re: RFR: 8277992: Add fast jdk_svc subtests to jdk:tier3
On Tue, 30 Nov 2021 18:48:15 GMT, Aleksey Shipilev wrote: > OpenJDK tiered tests definitions have the catch-all `tier4` that runs all > tests not defined in the lower tiers. `hotspot:tier4` has lots of them, > mostly long-running vmTestbase tests, which take many hours even on a very > parallel machines. > > This, unfortunately, has a chilling effect on `jdk:tier4`, which is seldom > run by contributors, because `hotspot:tier4` is in the way. But, there are > plenty of fast and stable tests in `jdk:tier4` that can be run in > `jdk:tier3`. `jdk_svc` is the example of such tests: management features > (including but not limited to JFR) are important to keep from regressions, > and significant subset of them runs pretty fast. > > So, it makes sense to move some `jdk_svc` tests to `jdk:tier3` to expose it > to more contributors. I think the only group we don't want to run is > `svc_tools`, which includes lots of non-parallel tests that are rather slow. > > Sample run before: > > > == > Test summary > == >TEST TOTAL PASS FAIL ERROR > >jtreg:test/jdk:tier3174 174 0 0 > > == > TEST SUCCESS > > Finished building target 'run-test' in configuration > 'linux-x86_64-server-fastdebug' > > real 2m38.242s > user 80m7.216s > sys 2m13.846s > > > == > Test summary > == >TEST TOTAL PASS FAIL ERROR > >>> jtreg:test/jdk:tier4 2904 2901 3 0 << > == > TEST FAILURE > > real 18m13.933s > user 546m50.556s > sys 25m7.086s > > > Sample run after: > > > == > Test summary > == >TEST TOTAL PASS FAIL ERROR > >jtreg:test/jdk:tier3 1296 1296 0 0 > > == > TEST SUCCESS > > Finished building target 'run-test' in configuration > 'linux-x86_64-server-fastdebug' > > real 7m49.017s > user 287m30.943s > sys 13m20.060s > > == > Test summary > == >TEST TOTAL PASS FAIL ERROR > >>> jtreg:test/jdk:tier4 1783 1780 3 0 << > == > TEST FAILURE > > > real 12m19.973s > user 351m48.561s > sys 14m51.566s Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6619
Re: RFR: 8277992: Add fast jdk_svc subtests to jdk:tier3
On Mon, 6 Dec 2021 09:11:07 GMT, Aleksey Shipilev wrote: >>> I've solicited feedback from core-libs folk as this affects them the most. >>> At present we, Oracle, run the jdk_svc tests as part of hotspot testing, >>> but this change will suddenly cause jdk testing to include them. That is >>> probably not an issue for testing within the CI framework but developers >>> running jdk_tier3 locally, often, may have an unpleasant surprise at >>> running over a thousand additional tests that probably have zero relevance >>> to what they are trying to test. Local machines may take considerably >>> longer than the 8+ minutes that you listed. >> >> I don't expect there are many people that run jdk_tier3 locally. It's a >> mixed bag of tests for the Panama Vector API, RMI and JFR. Adding jdk_svc to >> this list just adds to the mix. The only synergy is that the RMI tests will >> be in the same tier as the JMX tests that use the RMI connectors. No >> objection to the change, I think it is just a re-balancing of tiers for CI >> systems. > >> No objection to the change, I think it is just a re-balancing of tiers for >> CI systems. > > Yes, quite. @dholmes-ora, are you happy with Alan's assessment? @shipilev yes I'm fine with Alan's assessment. Thanks - PR: https://git.openjdk.java.net/jdk/pull/6619
Re: RFR: JDK-8278273: Remove unnecessary exclusion of doclint accessibility checks
On Sun, 5 Dec 2021 23:45:32 GMT, Joe Darcy wrote: > Exploratory builds indicate it is not currently necessary to exclude the > doclint accessibility checks; this patch enables them. > > (Enabling the reference checks is left for future work.) - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6713
Re: RFR: 8278185: Custom JRE cannot find non-ASCII named module inside
On Fri, 3 Dec 2021 07:29:17 GMT, Toshio Nakamura wrote: > Could you review this fix? > > Problem: > Custom JRE generated by jlink cannot find non-ASCII named modules included > inside the JRE. > > Cause and fix: > If module or package name was composed by ASCII then non-ASCII characters, > ImageStringsReader:stringFromByteBufferMatches() miscalculated the length of > matched string. The first part of ASCII characters was missing. This patch > corrected the value. > > Testing: > tier1 and tier2 on Linux have no regression. > I wasn't able to create an automate test for this issue. I appreciate any > advice. Yes, I think we would be good to have a test. If it's too hard using the tools then a test that uses the image reader API directly would be okay. @naotoj may have some suggestions too. - PR: https://git.openjdk.java.net/jdk/pull/6693
Re: RFR: 8277992: Add fast jdk_svc subtests to jdk:tier3
On Fri, 3 Dec 2021 07:34:16 GMT, Alan Bateman wrote: > No objection to the change, I think it is just a re-balancing of tiers for CI > systems. Yes, quite. @dholmes-ora, are you happy with Alan's assessment? - PR: https://git.openjdk.java.net/jdk/pull/6619
Re: RFR: 8265891: (ch) InputStream returned by Channels.newInputStream should override transferTo [v13]
On Mon, 6 Dec 2021 07:07:03 GMT, Markus KARG wrote: >> Markus KARG has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Draft: Eliminated duplicate code using lambda expressions >> - Draft: Use blocking mode also for target channel > > Please keep this PR open. More commits will follow on the next weeks. Looks promising, keep moving, @mkarg ! - PR: https://git.openjdk.java.net/jdk/pull/4263
