Re: RFR: 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed [v2]
> 8323640: [TESTBUG]testMemoryFailCount in > jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because > OOM killed sendaoYan has updated the pull request incrementally with one additional commit since the last revision: 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed Signed-off-by: sendaoYan - Changes: - all: https://git.openjdk.org/jdk/pull/17514/files - new: https://git.openjdk.org/jdk/pull/17514/files/be81665d..969b608d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17514&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17514&range=00-01 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/17514.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17514/head:pull/17514 PR: https://git.openjdk.org/jdk/pull/17514
Re: RFR: 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed
On Mon, 22 Jan 2024 15:03:18 GMT, Severin Gehwolf wrote: > `1k` increments for a total of `512k` times seems overkill. Are you sure > that's needed to make the test pass? How about `1MB` increments for a total > of `512` times? When the docker serivice work normally on the test machine, this test will always fail. This test want to verify the API `jdk.internal.platform.Metrics.systemMetrics().getMemoryFailCount()` work normally or not. The API return memory allocate fail times in jvm. But, before this PR, everytime it allocate `1M` memory, the API has no chance the catch the memory allocate fail, the jvm was killed by OOM. Change `8M` increments to `1K` mean to avoid OOM killed for the jvm in docker container. jvm was killed by OOM in docker container: ![image](https://github.com/openjdk/jdk/assets/24123821/c00697cc-ceef-410e-a8b9-7c401fa76134) `1M` Increnents also can avoid OOM killed. ![image](https://github.com/openjdk/jdk/assets/24123821/bab0a753-d15c-4759-a557-b8feafaa97cb) - PR Comment: https://git.openjdk.org/jdk/pull/17514#issuecomment-1905139487
Re: RFR: 8323717: Introduce test keyword for tests that need external dependencies
On Mon, 15 Jan 2024 10:48:23 GMT, Aleksey Shipilev wrote: > Some jtreg tests require resolvable external dependencies. This resolution is > delegated to JIB, which is not used in vanilla OpenJDK testing. It would be > convenient to add a keyword that marks tests that require these external > dependencies, so that we could exclude those tests from runs. This would > allow us to: a) run all tests in hotspot:tier4, which now excludes > `applications/` specifically; b) make all tests runs (#17422) cleaner on many > environments. > > I provisionally call this flag `external-dep`, but I am open for other > suggestions. > > Note that some tests that pull `@Artifact`-s provide special paths that do > limited testing anyway. However, there are tests which cannot run without > external dependencies at all. These include at least `applications/jcstress` > and `applications/scimark` tests. > > Ironically, I cannot run the jcstress test generator because the dependencies > are lacking here. I regenerated those test using a self-built jcstress 0.16 > bundle. > > Additional testing: > - [x] `make test TEST=applications/` fails > - [x] `JTREG_KEYWORDS=!external-dep make test TEST=applications/` passes, > skipping most of the tests Seems quite reasonable. - Marked as reviewed by dholmes (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17421#pullrequestreview-1837713758
Re: RFR: 8303866: Allow ZipInputStream.readEnd to parse small Zip64 ZIP files [v12]
On Mon, 22 Jan 2024 14:34:25 GMT, Eirik Bjørsnøs wrote: > @LanceAndersen Do you have a cent or two to spare? Let me try and dig out from a couple of things and circle back to this again - PR Review Comment: https://git.openjdk.org/jdk/pull/12524#discussion_r1462519461
Re: RFR: JDK-8321545: Override toString() for Format subclasses [v4]
On Fri, 12 Jan 2024 21:30:05 GMT, Roger Riggs wrote: >> Justin Lu 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: >> >> - replace 'None' with 'null' for applicable classes >> - Merge branch 'master' into JDK-8321545-toString-j.text.Format >> - swap placement of decimal pattern and compact patterns. Expand on tests >> - add unit tests >> - Merge branch 'master' into JDK-8321545-toString-j.text.Format >> - account for null locale for SDF through deserialization >> - Merge branch 'master' into JDK-8321545-toString-j.text.Format >> - init > > src/java.base/share/classes/java/text/MessageFormat.java line 1195: > >> 1193: """ >> 1194: MessageFormat [locale: "%s", pattern: "%s"] >> 1195: """.formatted(locale == null ? "null" : >> locale.getDisplayName(), toPattern()); > > It would be more accurate if when locale ==null that null was not quoted in > the string. > Seeing "null" would imply that the displayName of the locale was "null", when > it was `null`. Hi Roger, addressed in https://github.com/openjdk/jdk/pull/17355/commits/70e0a175037ccd0215d76fb2fbfa8c91de291d41; would like to confirm the update is okay with you before integration. - PR Review Comment: https://git.openjdk.org/jdk/pull/17355#discussion_r1462516181
Re: RFR: 8322149: ConcurrentHashMap smarter presizing for copy constructor and putAll [v3]
On Mon, 22 Jan 2024 17:45:45 GMT, Volker Simonis wrote: >> Joshua Cao has updated the pull request incrementally with one additional >> commit since the last revision: >> >> putAll presize based on sum on both map sizes > > test/micro/org/openjdk/bench/java/util/concurrent/Maps.java line 122: > >> 120: @Benchmark >> 121: public ConcurrentHashMap >> testConcurrentHashMapPutAll() { >> 122: ConcurrentHashMap map = new >> ConcurrentHashMap<>(); > > I think this benchmark could be made more accurate by creating the new, > temporary map with the right initial size (i.e. `ConcurrentHashMap<>(nkeys)`) > to avoid calls to `tryPresize()` in this setup step. I updated. The numbers are surprisingly the same. I guess the benchmark compute time is dominated by putAll(). - PR Review Comment: https://git.openjdk.org/jdk/pull/17116#discussion_r1462481286
Re: RFR: 8322149: ConcurrentHashMap smarter presizing for copy constructor and putAll [v4]
> ConcurrentHashMap's copy constructor calls `putAll()` -> `tryPresize()` -> > `transfer()`. When coming from the copy constructor, the Map is empty, so > there is nothing to transfer. But `transfer()` will still copy all the empty > nodes from the old table to the new table. > > This patch avoids this work by only calling `tryPresize()` if the table is > already initialized. If `table` is null, the initialization is deferred to > `putVal()`, which calls `initTable()`. > > --- > > ### JMH results for testCopyConstructor > > before patch: > > > Result "org.openjdk.bench.java.util.concurrent.Maps.testCopyConstructor": > 937395.686 ±(99.9%) 99074.324 ns/op [Average] > (min, avg, max) = (825732.550, 937395.686, 1072024.041), stdev = 92674.184 > CI (99.9%): [838321.362, 1036470.010] (assumes normal distribution) > > > after patch: > > > Result "org.openjdk.bench.java.util.concurrent.Maps.testCopyConstructor": > 620871.469 ±(99.9%) 59195.406 ns/op [Average] > (min, avg, max) = (545304.633, 620871.469, 689013.573), stdev = 55371.419 > CI (99.9%): [561676.063, 680066.875] (assumes normal distribution) > > > Average time is decreased by about 33%. > > ### JMH results for testPutAll (size = 1) > > before patch: > > > Result > "org.openjdk.bench.java.util.concurrent.Maps.testConcurrentHashMapPutAll": > 4315291.542 ±(99.9%) 336034.190 ns/op [Average] > (min, avg, max) = (3974688.194, 4315291.542, 4871772.209), stdev = > 314326.589 > CI (99.9%): [3979257.352, 4651325.731] (assumes normal distribution) > > > after patch: > > > Result > "org.openjdk.bench.java.util.concurrent.Maps.testConcurrentHashMapPutAll": > 3006955.723 ±(99.9%) 271757.969 ns/op [Average] > (min, avg, max) = (2801264.198, 3006955.723, 3553084.135), stdev = > 254202.573 > CI (99.9%): [2735197.754, 3278713.692] (assumes normal distribution) > > > Average time is decreased about 30%. Joshua Cao has updated the pull request incrementally with one additional commit since the last revision: pass initial size to putAll benchmark map construction - Changes: - all: https://git.openjdk.org/jdk/pull/17116/files - new: https://git.openjdk.org/jdk/pull/17116/files/3efa593d..2a965fa9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17116&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17116&range=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17116.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17116/head:pull/17116 PR: https://git.openjdk.org/jdk/pull/17116
Re: RFR: JDK-8323186: A faster algorithm for MutablebigInteger.divWord(long, int) [v4]
On Mon, 22 Jan 2024 21:26:39 GMT, Brian Burkhalter wrote: >> @bplb It is not used in jtreg testing. > > So it is only for verification purposes in the context of this PR? @bplb Yes, it is. - PR Review Comment: https://git.openjdk.org/jdk/pull/17291#discussion_r1462444386
Re: RFR: JDK-8323186: A faster algorithm for MutablebigInteger.divWord(long, int) [v4]
On Mon, 22 Jan 2024 21:21:53 GMT, fabioromano1 wrote: >> test/jdk/java/math/BigInteger/TestDivWord.java line 5: >> >>> 3: import java.util.Random; >>> 4: >>> 5: public class TestDivWord { >> >> Where is this used in actually jtreg testing? > > @bplb It is not used in jtreg testing. So it is only for verification purposes in the context of this PR? - PR Review Comment: https://git.openjdk.org/jdk/pull/17291#discussion_r1462441622
Re: RFR: JDK-8323186: A faster algorithm for MutablebigInteger.divWord(long, int) [v4]
On Mon, 22 Jan 2024 20:02:34 GMT, Brian Burkhalter wrote: >> fabioromano1 has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Update TestDivWord.java >> >> Added method to check results of divWord > > test/jdk/java/math/BigInteger/TestDivWord.java line 5: > >> 3: import java.util.Random; >> 4: >> 5: public class TestDivWord { > > Where is this used in actually jtreg testing? @bplb It is not used in jtreg testing. - PR Review Comment: https://git.openjdk.org/jdk/pull/17291#discussion_r1462437309
Re: RFR: 8320759: Creation of random BigIntegers can be made faster [v3]
On Sat, 2 Dec 2023 15:42:18 GMT, fabioromano1 wrote: >> A faster and simpler way to generate random BigIntegers, avoiding eventually >> trimming of leading zeros in magnitude array. > > fabioromano1 has updated the pull request incrementally with one additional > commit since the last revision: > > Create RandomBigIntegers.java > > Create a benchmark to measure the performance of BigInteger(int, Random) > constructor implementation. Will more work be done on this PR? - PR Comment: https://git.openjdk.org/jdk/pull/16817#issuecomment-1904812784
Re: RFR: JDK-8323186: A faster algorithm for MutablebigInteger.divWord(long, int) [v4]
On Mon, 22 Jan 2024 18:52:43 GMT, fabioromano1 wrote: >> The method `MutableBigInteger.divWord(long, int)` can use the algorithm of >> Hacker's Delight (2nd ed), section 9.3, the same used in >> `Long.divideUnsigned(long, long)` and `Long.remainderUnsigned(long, long)`, >> to get the computation faster. > > fabioromano1 has updated the pull request incrementally with one additional > commit since the last revision: > > Update TestDivWord.java > > Added method to check results of divWord test/jdk/java/math/BigInteger/TestDivWord.java line 5: > 3: import java.util.Random; > 4: > 5: public class TestDivWord { Where is this used in actually jtreg testing? - PR Review Comment: https://git.openjdk.org/jdk/pull/17291#discussion_r1462355967
Re: RFR: 8322768: Optimize non-subword vector compress and expand APIs for AVX2 target. [v8]
On Sat, 20 Jan 2024 09:55:45 GMT, Jatin Bhateja wrote: >> Hi, >> >> Patch optimizes non-subword vector compress and expand APIs for x86 AVX2 >> only targets. >> Upcoming E-core Xeons (Sierra Forest) and Hybrid CPUs only support AVX2 >> instruction set. >> These are very frequently used APIs in columnar database filter operation. >> >> Implementation uses a lookup table to record permute indices. Table index is >> computed using >> mask argument of compress/expand operation. >> >> Following are the performance number of JMH micro included with the patch. >> >> >> System : Intel(R) Xeon(R) Platinum 8480+ (Sapphire Rapids) >> >> Baseline: >> Benchmark (size) Mode CntScore >> Error Units >> ColumnFilterBenchmark.filterDoubleColumn1024 thrpt2 142.767 >> ops/ms >> ColumnFilterBenchmark.filterDoubleColumn2047 thrpt2 71.436 >> ops/ms >> ColumnFilterBenchmark.filterDoubleColumn4096 thrpt2 35.992 >> ops/ms >> ColumnFilterBenchmark.filterFloatColumn 1024 thrpt2 182.151 >> ops/ms >> ColumnFilterBenchmark.filterFloatColumn 2047 thrpt2 91.096 >> ops/ms >> ColumnFilterBenchmark.filterFloatColumn 4096 thrpt2 44.757 >> ops/ms >> ColumnFilterBenchmark.filterIntColumn 1024 thrpt2 184.099 >> ops/ms >> ColumnFilterBenchmark.filterIntColumn 2047 thrpt2 91.981 >> ops/ms >> ColumnFilterBenchmark.filterIntColumn 4096 thrpt2 45.170 >> ops/ms >> ColumnFilterBenchmark.filterLongColumn 1024 thrpt2 148.017 >> ops/ms >> ColumnFilterBenchmark.filterLongColumn 2047 thrpt2 73.516 >> ops/ms >> ColumnFilterBenchmark.filterLongColumn 4096 thrpt2 36.844 >> ops/ms >> >> Withopt: >> Benchmark (size) Mode Cnt Score >> Error Units >> ColumnFilterBenchmark.filterDoubleColumn1024 thrpt2 2051.707 >>ops/ms >> ColumnFilterBenchmark.filterDoubleColumn2047 thrpt2 914.072 >>ops/ms >> ColumnFilterBenchmark.filterDoubleColumn4096 thrpt2 489.898 >>ops/ms >> ColumnFilterBenchmark.filterFloatColumn 1024 thrpt2 5324.195 >>ops/ms >> ColumnFilterBenchmark.filterFloatColumn 2047 thrpt2 2587.229 >>ops/ms >> ColumnFilterBenchmark.filterFloatColumn 4096 thrpt2 1278.665 >>ops/ms >> ColumnFilterBenchmark.filterIntColumn 1024 thrpt2 4149.384 >>ops/ms >> ColumnFilterBenchmark.filterIntColumn 2047 thrpt ... > > Jatin Bhateja has updated the pull request incrementally with one additional > commit since the last revision: > > Review comments resolution Nice work! The patch looks good to me now. - Marked as reviewed by sviswanathan (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17261#pullrequestreview-1837250879
Re: Gatherer: spliterator characteristics are not propagated
> From: "Viktor Klang" > To: "Remi Forax" > Cc: "core-libs-dev" , "Paul Sandoz" > > Sent: Monday, January 22, 2024 4:22:11 PM > Subject: Re: Gatherer: spliterator characteristics are not propagated > Hi Rémi, Hello, > For instance, stateless is neither recessive nor dominant, since the > composition > of two stateless operations is only ever stateless if they both are greedy as > well: [ > https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/stream/Gatherers.java#L588 > | > https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/stream/Gatherers.java#L588 > ] Okay, so choosing SEQUENTIAL vs PARALELLIZABLE is not that important given that the combination is ad-hoc, reflecting the characterristics is enough. > So even if making it represented as ints (more like Spliterator, rather than > Collector) makes things faster, it's still both work to track, propagate, and > also becomes a side-channel that needs to remain in sync with the actual > implementation of the logic. The flags are in sync with the implementation because the only way to create a Gatherer if through the factory methods and those factory methods (and only them) compute the proper combination of SEQUENTIAL | STATELESS | GREEDY. A user should not be able to set those flags. Only the flags KEEP_* are settable by a user. > One could argue that logic such as: someCollection.stream().map(…).count() is > a > performance bug/inefficiency in an of itself as it would be faster to do > someCollection.size(). The stream implementation has a whole mechanism in place to propagate/preverse flags like SIZED, DISTINCT or SORTED. For me, discussing about the merit of this mechanism seems a little off topic. I would prefer the Gatherer to be a good citizen and works seemlessly with the other intermediary operations. > Cheers, > √ regards, Rémi > Viktor Klang > Software Architect, Java Platform Group > Oracle > From: fo...@univ-mlv.fr > Sent: Saturday, 20 January 2024 17:40 > To: Viktor Klang > Cc: core-libs-dev ; Paul Sandoz > > Subject: [External] : Re: Gatherer: spliterator characteristics are not > propagated >> From: "Viktor Klang" >> To: "Remi Forax" >> Cc: "core-libs-dev" , "Paul Sandoz" >> >> Sent: Thursday, January 18, 2024 5:14:38 PM >> Subject: Re: Gatherer: spliterator characteristics are not propagated >>> And for A.andThen(B), A.flags & B.flags should work, the stream is sorted if >>> both gatherers keep it sorted. >> That is unfortunately not the case. That would presume that you can implement >> the composition such that it can preserve all the common flags. Some flags >> could be "dominant" and some "recessive" to use genetics nomenclature. > Some flags of the stream pipeline are "recessive", mainly SHORT_CIRCUIT, but > not > the characteristics of the Gatherer which can have the corresponding > "dominant" > flag, GREEDY, in this case. > And the same for sequential, the flag should be PARALELIZABLE and not > SEQUENTIAL. > The idea is that the Gatherer characteristics can have the same bit set at the > same position as the stream op flags (as defined by StreamOpFlag). > So KEEP_DISTINCT is in position 0, KEEP_SORTED in in position 2 and KEEP_SIZED > is in position 3. > For GREEDY, we use the same position as SHORT_CIRCUIT and we will flip the bit > (using XOR) when we want to extract the stream op flags from the > characteristics > All the factory methods call the generic of() with a combination of > PARALELIZABLE and STATELESS and the user can adds the characteristics GREEDY, > KEEP_DISTINCT, KEEP_SORTED and KEEP_SIZED (otherwise an exception should be > raised). > In StreamOpFlag, there are two unused positions (14 and 15), that's perfect > for > our two new states PARALELIZABLE and STATELESS, so no problem here > (technically > we can also reuse positions of the Spliterator characteristic given that those > flags are masked before being sent to the GathererOp super constructor). > The way to transform a Gatherer characteristics op to a stream flags op is to > flip the bits corresponding to SHORT_CIRCUIT, add the highter bit of all flags > but SHORT-CIRCUIT (because stream op flags are encoded using 2 bits) and mask > to only retain SHORT_CIRCUIT, KEEP_DISTINCT, KEEP_SORTED and KEEP_SIZED. > public static int toOpFlags ( int characteristics ) { > return (( characteristics ^ SHORT_CIRCUIT_MASK ) | HIGHER_BITS ) & > STREAM_OP_MASK ; > } > see below for a full script. >>> I suppose that if those flags already exist, it's because they have a >>> purpose >>> and i do not understand how it can make the other operations slower. >> Extra invocations, extra storage, and extra composition overhead is not free. >> Since Stream is one-shot you need to include the construction cost with the >> execution cost. For something like an empty Stream construction cost scan be >> 90+% of the total costs. > If you create a Gatherer, the characteristics is a con
Re: RFR: JDK-8323186: A faster algorithm for MutablebigInteger.divWord(long, int) [v3]
On Mon, 22 Jan 2024 18:07:49 GMT, Raffaello Giulietti wrote: >> fabioromano1 has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Removed trailing whitespaces > > As you note, that would probably require two divisions. I don't know if the > JIT compiler can detect that the arguments are the same and emit just one > division instead. > I think your code is good enough for the purpose of [Mutable]BigInteger, and > better than the current one. @rgiulietti I've added a method to check the results of tests. - PR Comment: https://git.openjdk.org/jdk/pull/17291#issuecomment-1904607454
Re: RFR: JDK-8323186: A faster algorithm for MutablebigInteger.divWord(long, int) [v4]
> The method `MutableBigInteger.divWord(long, int)` can use the algorithm of > Hacker's Delight (2nd ed), section 9.3, the same used in > `Long.divideUnsigned(long, long)` and `Long.remainderUnsigned(long, long)`, > to get the computation faster. fabioromano1 has updated the pull request incrementally with one additional commit since the last revision: Update TestDivWord.java Added method to check results of divWord - Changes: - all: https://git.openjdk.org/jdk/pull/17291/files - new: https://git.openjdk.org/jdk/pull/17291/files/2460b066..67e3027f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17291&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17291&range=02-03 Stats: 14 lines in 1 file changed: 11 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/17291.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17291/head:pull/17291 PR: https://git.openjdk.org/jdk/pull/17291
Re: RFR: 8323835: Updating ASM to 9.6 for JDK 23
On Tue, 16 Jan 2024 21:18:55 GMT, Vicente Romero wrote: > Updating ASM to version 9.6, > > Thanks in advance for the reviews, > Vicente The change looks okay to me. Most of the changes are doc change. I see many files are updated just to remove the empty line at the end of the file - is that intentional, imported from this upgrade? Any need to update the copyright header? The new file CheckFrameAnalyzer.java still has the same copyright header years as other files. I assume no need then? - PR Comment: https://git.openjdk.org/jdk/pull/17453#issuecomment-1904588248
Re: RFR: 8303374: Compiler Implementation for Primitive types in patterns, instanceof, and switch (Preview) [v41]
> This is the proposed patch for Primitive types in patterns, instanceof, and > switch (Preview). > > Draft spec here: https://cr.openjdk.org/~abimpoudis/instanceof/latest/ Aggelos Biboudis has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 60 commits: - Improve Javadoc of ExactConversionsSupport - Merge branch 'master' into primitive-patterns - Update ExactConversionsSupport Javadoc and JDK version - Revert "Update copyright year, javadoc, JDK version" This reverts commit 676af9de90d946f64f34762a6df94dbd91bce41b. - Update copyright year, javadoc, JDK version - Merge branch 'master' into primitive-patterns - Merge branch 'master' into primitive-patterns - Cleanup - Merge branch 'master' into primitive-patterns - Remove trailing spaces - ... and 50 more: https://git.openjdk.org/jdk/compare/c9cacfb2...50cb9832 - Changes: https://git.openjdk.org/jdk/pull/15638/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=15638&range=40 Stats: 3272 lines in 41 files changed: 3008 ins; 110 del; 154 mod Patch: https://git.openjdk.org/jdk/pull/15638.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15638/head:pull/15638 PR: https://git.openjdk.org/jdk/pull/15638
Re: RFR: JDK-8323186: A faster algorithm for MutablebigInteger.divWord(long, int) [v3]
On Mon, 22 Jan 2024 18:07:49 GMT, Raffaello Giulietti wrote: >> fabioromano1 has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Removed trailing whitespaces > > As you note, that would probably require two divisions. I don't know if the > JIT compiler can detect that the arguments are the same and emit just one > division instead. > I think your code is good enough for the purpose of [Mutable]BigInteger, and > better than the current one. @rgiulietti It seems unlikely that the JIT compiler can do that, at least to me. - PR Comment: https://git.openjdk.org/jdk/pull/17291#issuecomment-1904544724
Re: RFR: JDK-8323186: A faster algorithm for MutablebigInteger.divWord(long, int) [v3]
On Tue, 9 Jan 2024 18:24:49 GMT, fabioromano1 wrote: >> The method `MutableBigInteger.divWord(long, int)` can use the algorithm of >> Hacker's Delight (2nd ed), section 9.3, the same used in >> `Long.divideUnsigned(long, long)` and `Long.remainderUnsigned(long, long)`, >> to get the computation faster. > > fabioromano1 has updated the pull request incrementally with one additional > commit since the last revision: > > Removed trailing whitespaces As you note, that would probably require two divisions. I don't know if the JIT compiler can detect that the arguments are the same and emit just one division instead. I think your code is good enough for the purpose of [Mutable]BigInteger, and better than the current one. - PR Comment: https://git.openjdk.org/jdk/pull/17291#issuecomment-1904538746
Re: RFR: 8322149: ConcurrentHashMap smarter presizing for copy constructor and putAll [v3]
On Wed, 17 Jan 2024 21:16:02 GMT, Joshua Cao wrote: >> ConcurrentHashMap's copy constructor calls `putAll()` -> `tryPresize()` -> >> `transfer()`. When coming from the copy constructor, the Map is empty, so >> there is nothing to transfer. But `transfer()` will still copy all the empty >> nodes from the old table to the new table. >> >> This patch avoids this work by only calling `tryPresize()` if the table is >> already initialized. If `table` is null, the initialization is deferred to >> `putVal()`, which calls `initTable()`. >> >> --- >> >> ### JMH results for testCopyConstructor >> >> before patch: >> >> >> Result "org.openjdk.bench.java.util.concurrent.Maps.testCopyConstructor": >> 937395.686 ±(99.9%) 99074.324 ns/op [Average] >> (min, avg, max) = (825732.550, 937395.686, 1072024.041), stdev = 92674.184 >> CI (99.9%): [838321.362, 1036470.010] (assumes normal distribution) >> >> >> after patch: >> >> >> Result "org.openjdk.bench.java.util.concurrent.Maps.testCopyConstructor": >> 620871.469 ±(99.9%) 59195.406 ns/op [Average] >> (min, avg, max) = (545304.633, 620871.469, 689013.573), stdev = 55371.419 >> CI (99.9%): [561676.063, 680066.875] (assumes normal distribution) >> >> >> Average time is decreased by about 33%. >> >> ### JMH results for testPutAll (size = 1) >> >> before patch: >> >> >> Result >> "org.openjdk.bench.java.util.concurrent.Maps.testConcurrentHashMapPutAll": >> 4315291.542 ±(99.9%) 336034.190 ns/op [Average] >> (min, avg, max) = (3974688.194, 4315291.542, 4871772.209), stdev = >> 314326.589 >> CI (99.9%): [3979257.352, 4651325.731] (assumes normal distribution) >> >> >> after patch: >> >> >> Result >> "org.openjdk.bench.java.util.concurrent.Maps.testConcurrentHashMapPutAll": >> 3006955.723 ±(99.9%) 271757.969 ns/op [Average] >> (min, avg, max) = (2801264.198, 3006955.723, 3553084.135), stdev = >> 254202.573 >> CI (99.9%): [2735197.754, 3278713.692] (assumes normal distribution) >> >> >> Average time is decreased about 30%. > > Joshua Cao has updated the pull request incrementally with one additional > commit since the last revision: > > putAll presize based on sum on both map sizes Thanks for the latest changes. The patch looks good now, except that I think the benchmark could be made more accurate (see inline comments). This will probably result in an even higher improvement for `putAll()`. test/micro/org/openjdk/bench/java/util/concurrent/Maps.java line 122: > 120: @Benchmark > 121: public ConcurrentHashMap > testConcurrentHashMapPutAll() { > 122: ConcurrentHashMap map = new > ConcurrentHashMap<>(); I think this benchmark could be made more accurate by creating the new, temporary map with the right initial size (i.e. `ConcurrentHashMap<>(nkeys)`) to avoid calls to `tryPresize()` in this setup step. - Changes requested by simonis (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17116#pullrequestreview-1837027167 PR Review Comment: https://git.openjdk.org/jdk/pull/17116#discussion_r1462205109
Re: RFR: JDK-8323186: A faster algorithm for MutablebigInteger.divWord(long, int) [v3]
On Mon, 22 Jan 2024 17:23:22 GMT, Raffaello Giulietti wrote: > AFAIK, IntrinsicCandidate methods are only relevant in JIT compiled code. A > test that checks correctness might not reach the compilation stage, and > execute only in the bytecode interpreter. > > But IMO using the result of `divWord()` as suggested is simple enough and > avoids hidden dependencies. @rgiulietti Perhaps I was not clear enough. I'm not intended to use `Long.divideUnsigned(long, long)` and `Long.remainderUnsigned(long, long)` in the tests, but in the algorithm implementation, instead of using an hard-coded implementation, to use directly the assembly code, although this implies to calculate the approximations of `q` and `r` two times instead of one. - PR Comment: https://git.openjdk.org/jdk/pull/17291#issuecomment-1904488957
Re: RFR: JDK-8323186: A faster algorithm for MutablebigInteger.divWord(long, int) [v3]
On Tue, 9 Jan 2024 18:24:49 GMT, fabioromano1 wrote: >> The method `MutableBigInteger.divWord(long, int)` can use the algorithm of >> Hacker's Delight (2nd ed), section 9.3, the same used in >> `Long.divideUnsigned(long, long)` and `Long.remainderUnsigned(long, long)`, >> to get the computation faster. > > fabioromano1 has updated the pull request incrementally with one additional > commit since the last revision: > > Removed trailing whitespaces AFAIK, IntrinsicCandidate methods are only relevant in JIT compiled code. A test that checks correctness might not reach the compilation stage, and execute only in the bytecode interpreter. But IMO using the result of `divWord()` as suggested is simple enough and avoids hidden dependencies. - PR Comment: https://git.openjdk.org/jdk/pull/17291#issuecomment-1904466691
Integrated: 8324065: Daylight saving information for `Africa/Casablanca` are incorrect
On Thu, 18 Jan 2024 19:37:33 GMT, Naoto Sato wrote: > Fixing incorrect std/dst transitions for time zones that have rules beyond > 2037. The year 2037 restriction seems to come from the old `zic` command > implementation and thus can be expanded in the JDK. Arbitrary I chose 2100 > which is greater than the year 2087 which is the farthest rule at the moment. This pull request has now been integrated. Changeset: 0d8543d6 Author:Naoto Sato URL: https://git.openjdk.org/jdk/commit/0d8543d6773a516dad54038070dce507179d0709 Stats: 44 lines in 6 files changed: 5 ins; 7 del; 32 mod 8324065: Daylight saving information for `Africa/Casablanca` are incorrect Reviewed-by: iris, joehw, jlu - PR: https://git.openjdk.org/jdk/pull/17492
Re: RFR: JDK-8323186: A faster algorithm for MutablebigInteger.divWord(long, int) [v3]
On Mon, 22 Jan 2024 16:15:57 GMT, Raffaello Giulietti wrote: >> fabioromano1 has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Removed trailing whitespaces > > What about splitting the result of `divWord()` into its components `q` and > `r` and check that `0 <= r < d` and `n == q * d + r`? Mathematically this > should be sufficient. Perhaps you might need to take care about unsigned vs > signed arithmetic. @rgiulietti In this case, the unsigned arithmetic is not a problem, since `(d & LONG_MASK) > 0` and multiplication and addition semantics is equivalent in signed/unsigned integers. Perhaps, it would be better to call directly `Long.divideUnsigned(long, long)` and `Long.remainderUnsigned(long, long)` to possibly use directly assembly code, since they are annotated as IntrinsicCandidate. - PR Comment: https://git.openjdk.org/jdk/pull/17291#issuecomment-1904381418
Re: RFR: JDK-8323186: A faster algorithm for MutablebigInteger.divWord(long, int) [v3]
On Tue, 9 Jan 2024 18:24:49 GMT, fabioromano1 wrote: >> The method `MutableBigInteger.divWord(long, int)` can use the algorithm of >> Hacker's Delight (2nd ed), section 9.3, the same used in >> `Long.divideUnsigned(long, long)` and `Long.remainderUnsigned(long, long)`, >> to get the computation faster. > > fabioromano1 has updated the pull request incrementally with one additional > commit since the last revision: > > Removed trailing whitespaces What about splitting the result of `divWord()` into its components `q` and `r` and check that `0 <= r < d` and `n == q * d + r`? Mathematically this should be sufficient. Perhaps you might need to take care about unsigned vs signed arithmetic. - PR Comment: https://git.openjdk.org/jdk/pull/17291#issuecomment-1904341649
Re: RFR: JDK-8323186: A faster algorithm for MutablebigInteger.divWord(long, int) [v3]
On Mon, 22 Jan 2024 15:38:08 GMT, Raffaello Giulietti wrote: >> fabioromano1 has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Removed trailing whitespaces > > Sure. But the purpose of a test is not much to check the _current_ code but > to protect against erroneous changes in the future. Without a test, a later > modification might introduce an undetected regression. @rgiulietti By the way, the tests for correctness of `Long.divideUnsigned(long, long)` and `Long.remainderUnsigned(long, long)` simply check the consistency between their results and those of `BigInteger.divide(BigInteger)`, so those tests are implicitly tests also for `MutablebigInteger.divWord(long, int)`. - PR Comment: https://git.openjdk.org/jdk/pull/17291#issuecomment-1904311866
Re: RFR: JDK-8323186: A faster algorithm for MutablebigInteger.divWord(long, int) [v3]
On Mon, 22 Jan 2024 15:38:08 GMT, Raffaello Giulietti wrote: >> fabioromano1 has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Removed trailing whitespaces > > Sure. But the purpose of a test is not much to check the _current_ code but > to protect against erroneous changes in the future. Without a test, a later > modification might introduce an undetected regression. @rgiulietti The method `MutablebigInteger.divWord(long, int)` is used for division of BigIntegers, and division of BigIntegers already has several tests. Anyway, in case we can use the same tests which already exist for `Long.divideUnsigned(long, long)` and `Long.remainderUnsigned(long, long)`. - PR Comment: https://git.openjdk.org/jdk/pull/17291#issuecomment-1904279676
Re: RFR: JDK-8323186: A faster algorithm for MutablebigInteger.divWord(long, int) [v3]
On Tue, 9 Jan 2024 18:24:49 GMT, fabioromano1 wrote: >> The method `MutableBigInteger.divWord(long, int)` can use the algorithm of >> Hacker's Delight (2nd ed), section 9.3, the same used in >> `Long.divideUnsigned(long, long)` and `Long.remainderUnsigned(long, long)`, >> to get the computation faster. > > fabioromano1 has updated the pull request incrementally with one additional > commit since the last revision: > > Removed trailing whitespaces Sure. But the purpose of a test is not much to check the _current_ code but to protect against erroneous changes in the future. Without a test, a later modification might introduce an undetected regression. - PR Comment: https://git.openjdk.org/jdk/pull/17291#issuecomment-1904265345
Re: RFR: JDK-8323186: A faster algorithm for MutablebigInteger.divWord(long, int) [v3]
On Mon, 22 Jan 2024 15:14:58 GMT, Raffaello Giulietti wrote: >> fabioromano1 has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Removed trailing whitespaces > > The test does not seem to check that the results are correct, but just > measures elapsed time. > > For this, in the context of the OpenJDK, we use > [JMH](https://github.com/openjdk/jmh) for performance benchmarks. It avoids > all the common issues with less sophisticated approaches. At first, it might > seem harder to use, but it is more reliable than ad-hoc measurements. While > not perfect, it has established itself as the "golden standard", so to say. @rgiulietti The correctness of the algorithm simply follows by the fact that it is just a particularization of `Long.divideUnsigned(long, long)` and `Long.remainderUnsigned(long, long)`. - PR Comment: https://git.openjdk.org/jdk/pull/17291#issuecomment-1904256641
Re: Gatherer: spliterator characteristics are not propagated
Hi Rémi, For instance, stateless is neither recessive nor dominant, since the composition of two stateless operations is only ever stateless if they both are greedy as well: https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/stream/Gatherers.java#L588 So even if making it represented as ints (more like Spliterator, rather than Collector) makes things faster, it's still both work to track, propagate, and also becomes a side-channel that needs to remain in sync with the actual implementation of the logic. One could argue that logic such as: someCollection.stream().map(…).count() is a performance bug/inefficiency in an of itself as it would be faster to do someCollection.size(). Cheers, √ Viktor Klang Software Architect, Java Platform Group Oracle From: fo...@univ-mlv.fr Sent: Saturday, 20 January 2024 17:40 To: Viktor Klang Cc: core-libs-dev ; Paul Sandoz Subject: [External] : Re: Gatherer: spliterator characteristics are not propagated From: "Viktor Klang" To: "Remi Forax" Cc: "core-libs-dev" , "Paul Sandoz" Sent: Thursday, January 18, 2024 5:14:38 PM Subject: Re: Gatherer: spliterator characteristics are not propagated And for A.andThen(B), A.flags & B.flags should work, the stream is sorted if both gatherers keep it sorted. That is unfortunately not the case. That would presume that you can implement the composition such that it can preserve all the common flags. Some flags could be "dominant" and some "recessive" to use genetics nomenclature. Some flags of the stream pipeline are "recessive", mainly SHORT_CIRCUIT, but not the characteristics of the Gatherer which can have the corresponding "dominant" flag, GREEDY, in this case. And the same for sequential, the flag should be PARALELIZABLE and not SEQUENTIAL. The idea is that the Gatherer characteristics can have the same bit set at the same position as the stream op flags (as defined by StreamOpFlag). So KEEP_DISTINCT is in position 0, KEEP_SORTED in in position 2 and KEEP_SIZED is in position 3. For GREEDY, we use the same position as SHORT_CIRCUIT and we will flip the bit (using XOR) when we want to extract the stream op flags from the characteristics All the factory methods call the generic of() with a combination of PARALELIZABLE and STATELESS and the user can adds the characteristics GREEDY, KEEP_DISTINCT, KEEP_SORTED and KEEP_SIZED (otherwise an exception should be raised). In StreamOpFlag, there are two unused positions (14 and 15), that's perfect for our two new states PARALELIZABLE and STATELESS, so no problem here (technically we can also reuse positions of the Spliterator characteristic given that those flags are masked before being sent to the GathererOp super constructor). The way to transform a Gatherer characteristics op to a stream flags op is to flip the bits corresponding to SHORT_CIRCUIT, add the highter bit of all flags but SHORT-CIRCUIT (because stream op flags are encoded using 2 bits) and mask to only retain SHORT_CIRCUIT, KEEP_DISTINCT, KEEP_SORTED and KEEP_SIZED. public static int toOpFlags(int characteristics) { return ((characteristics ^ SHORT_CIRCUIT_MASK) | HIGHER_BITS) & STREAM_OP_MASK; } see below for a full script. I suppose that if those flags already exist, it's because they have a purpose and i do not understand how it can make the other operations slower. Extra invocations, extra storage, and extra composition overhead is not free. Since Stream is one-shot you need to include the construction cost with the execution cost. For something like an empty Stream construction cost scan be 90+% of the total costs. If you create a Gatherer, the characteristics is a constant (so the validity check is removed, it's just a mask and a test) so the result of calling toOpFlags() is a constant too. If the factory method is not inlined, the cost is 3 bitwise operations which is I believe faster than the "instanceof Greedy" used in the current code. Cheers, √ regards, Rémi --- public class GathererFlag { // cut and paste from StreamOpFlag /** * The bit pattern for setting/injecting a flag. */ private static final int SET_BITS = 0b01; /** * The bit pattern for clearing a flag. */ private static final int CLEAR_BITS = 0b10; /** * The bit pattern for preserving a flag. */ private static final int PRESERVE_BITS = 0b11; private static int position(int opFlagSet) { return Integer.numberOfTrailingZeros(opFlagSet) >> 1; } private static final int DISTINCT_POSITION = position(StreamOpFlag.IS_DISTINCT); private static final int SORTED_POSITION = position(StreamOpFlag.IS_SORTED); private static final int SIZED_POSITION = position(StreamOpFlag.IS_SIZED); private static final int SHORT_CIRCUIT_POSITION = position(StreamOpFlag.IS_SHORT_CIRCUIT); private static final int STATELESS_POSITION = 14; private static final int PARE
Re: RFR: JDK-8323186: A faster algorithm for MutablebigInteger.divWord(long, int) [v3]
On Tue, 9 Jan 2024 18:24:49 GMT, fabioromano1 wrote: >> The method `MutableBigInteger.divWord(long, int)` can use the algorithm of >> Hacker's Delight (2nd ed), section 9.3, the same used in >> `Long.divideUnsigned(long, long)` and `Long.remainderUnsigned(long, long)`, >> to get the computation faster. > > fabioromano1 has updated the pull request incrementally with one additional > commit since the last revision: > > Removed trailing whitespaces The test does not seem to check that the results are correct, but just measures elapsed time. For this, in the context of the OpenJDK, we use [JMH](https://github.com/openjdk/jmh) for performance benchmarks. It avoids all the common issues with less sophisticated approaches. At first, it might seem harder to use, but it is more reliable than ad-hoc measurements. While not perfect, it has established itself as the "golden standard", so to say. - PR Comment: https://git.openjdk.org/jdk/pull/17291#issuecomment-1904217509
Re: RFR: 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed
On Mon, 22 Jan 2024 09:31:43 GMT, sendaoYan wrote: > 8323640: [TESTBUG]testMemoryFailCount in > jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because > OOM killed `1k` increments for a total of `512k` times seems overkill. Are you sure that's needed to make the test pass? How about `1MB` increments for a total of `512` times? - PR Review: https://git.openjdk.org/jdk/pull/17514#pullrequestreview-1836685784
Re: RFR: 8303866: Allow ZipInputStream.readEnd to parse small Zip64 ZIP files [v12]
On Tue, 16 Jan 2024 14:55:39 GMT, Jaikiran Pai wrote: > I think the only role that a zip64 block should play when we are deciding how > to parse a data descriptor is whether or not the entry has zip64 extra field > set (the header id value of the extra field). Does that sound reasonable? Are you suggesting that we ONLY look for the presence of a Zip64 (tag 0x1) extended field? Meaning we should ignore the following: - The values of the 'compressed size' and 'uncompressed size' fields in the LOC header - The contents/fields in the data block in the Zip64 header The purpose of this check is to determine wheter the entry "is in the ZIP64 format", to use parlance from `APPNOTE.TXT`. The reason stronger validation was added was to avoid false positives, meaning an entry would be interpreted as "in the Zip64 format", even when that was not the intention of the producer. Some of this validation was on my initiative, but I think maybe some was in response to concerns raised in review. If we ignore the 'uncompressed size' and 'compresssed size' fields, then if a LOC has both of these set to zero and has a Zip64 extended field, then we'll intepret this as "in the Zip64 format", even though no field is marked using the Zip64 magic value `0x`. I think I would be fine with dropping inspection of the Zip64 contents (as there are probably efforts underway to add validation of LOC Zip64 fields similar to the recently added CEN validation). But I'm a bit worried that ignoring the LOC values will make us parse some data descriptors using 8 bytes, when the producer did in fact not intend to use the Zip64 format. (The Zip64 field just came through some kind of "pollution" or unintended copying. A natural way to implement Zip64 parsing is to only look at the extended field if at least one of the relevant LOC headers are `0x`, otherwise one can short-circuit and assume there is no Zip64 field. @LanceAndersen Do you have a cent or two to spare? - PR Review Comment: https://git.openjdk.org/jdk/pull/12524#discussion_r1461951695
Re: RFR: 8313739: ZipOutputStream.close() should always close the wrapped stream [v4]
On Tue, 16 Jan 2024 18:18:38 GMT, Lance Andersen wrote: > This comment could use a bit of wordsmithing to indicate what "correct" means It's hard to write good prose for these tricky error scenarios. But just saying "correct" without defining it is a bit too lazy, yes :-) Please take a look at the last commit 39a14de where I've expanded "correct" in the test comments. - PR Review Comment: https://git.openjdk.org/jdk/pull/17209#discussion_r1461905312
Re: RFR: 8313739: ZipOutputStream.close() should always close the wrapped stream [v5]
> Please consider this PR which makes `DeflaterOutputStream.close()` always > close its wrapped output stream exactly once. > > Currently, closing of the wrapped output stream happens outside the finally > block where `finish()` is called. If `finish()` throws, this means the > wrapped stream will not be closed. This can potentially lead to leaking > resources such as file descriptors or sockets. > > This fix is to move the closing of the wrapped stream inside the finally > block. > > Additionally, the `closed = true;` statement is moved to the start of the > close method. This makes sure we only ever close the wrapped stream once > (this aligns with the overridden method `FilterOutputStream.close´) > > Specification: This change brings the implementation of > `DeflaterOutputStream.close()` in line with its specification: *Writes > remaining compressed data to the output stream and closes the underlying > stream.* > > Risk: This is a behavioural change. There is a small risk that existing code > depends on the close method not following its specification. > > Testing: The PR adds a new JUnit 5 test `CloseWrappedStream.java` which > simulates the failure condition and verifies that the wrapped stream was > closed under failing and non-failing conditions. Eirik Bjørsnøs has updated the pull request incrementally with one additional commit since the last revision: Spell out what is checked by each test instead of using the word "correct" - Changes: - all: https://git.openjdk.org/jdk/pull/17209/files - new: https://git.openjdk.org/jdk/pull/17209/files/96deca07..39a14de3 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17209&range=04 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17209&range=03-04 Stats: 40 lines in 1 file changed: 33 ins; 1 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/17209.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17209/head:pull/17209 PR: https://git.openjdk.org/jdk/pull/17209
Re: RFR: 8322420: [Linux] cgroup v2: Limits in parent nested control groups are not detected [v3]
> The testcase requires root permissions. Jan Kratochvil 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 four additional commits since the last revision: - Merge branch 'master' into master-cgroup - Fix gtest testcases compilation errors - 8322420: [Linux] cgroup v2: Limits in parent nested control groups are not detected - 4c556a78: - Changes: - all: https://git.openjdk.org/jdk/pull/17198/files - new: https://git.openjdk.org/jdk/pull/17198/files/395ef61f..ae58f7e3 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17198&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17198&range=01-02 Stats: 44851 lines in 1274 files changed: 27136 ins; 11937 del; 5778 mod Patch: https://git.openjdk.org/jdk/pull/17198.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17198/head:pull/17198 PR: https://git.openjdk.org/jdk/pull/17198
Re: RFR: 8303374: Compiler Implementation for Primitive types in patterns, instanceof, and switch (Preview) [v40]
> This is the proposed patch for Primitive types in patterns, instanceof, and > switch (Preview). > > Draft spec here: https://cr.openjdk.org/~abimpoudis/instanceof/latest/ Aggelos Biboudis has updated the pull request incrementally with two additional commits since the last revision: - Update ExactConversionsSupport Javadoc and JDK version - Revert "Update copyright year, javadoc, JDK version" This reverts commit 676af9de90d946f64f34762a6df94dbd91bce41b. - Changes: - all: https://git.openjdk.org/jdk/pull/15638/files - new: https://git.openjdk.org/jdk/pull/15638/files/676af9de..ed61d5fd Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=15638&range=39 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=15638&range=38-39 Stats: 27 lines in 27 files changed: 0 ins; 0 del; 27 mod Patch: https://git.openjdk.org/jdk/pull/15638.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15638/head:pull/15638 PR: https://git.openjdk.org/jdk/pull/15638
Re: RFR: 8303374: Compiler Implementation for Primitive types in patterns, instanceof, and switch (Preview) [v39]
> This is the proposed patch for Primitive types in patterns, instanceof, and > switch (Preview). > > Draft spec here: https://cr.openjdk.org/~abimpoudis/instanceof/latest/ Aggelos Biboudis has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 56 commits: - Update copyright year, javadoc, JDK version - Merge branch 'master' into primitive-patterns - Merge branch 'master' into primitive-patterns - Cleanup - Merge branch 'master' into primitive-patterns - Remove trailing spaces - Merge branch 'primitive-patterns-and-generating-dispatch' into primitive-patterns - Fixed switch in the cases of unboxing and widening - Merge branch 'JDK-8319220' into primitive-patterns - Merge branch 'master' into JDK-8319220 - ... and 46 more: https://git.openjdk.org/jdk/compare/9049402a...676af9de - Changes: https://git.openjdk.org/jdk/pull/15638/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=15638&range=38 Stats: 3281 lines in 41 files changed: 3000 ins; 110 del; 171 mod Patch: https://git.openjdk.org/jdk/pull/15638.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15638/head:pull/15638 PR: https://git.openjdk.org/jdk/pull/15638
Re: RFR: 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed
On Mon, 22 Jan 2024 09:31:43 GMT, sendaoYan wrote: > 8323640: [TESTBUG]testMemoryFailCount in > jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because > OOM killed The test case before this PR has a maximum heap of 64MB and applies for 8M of memory each time in the for loop. When applying for memory for the sixth time, it was killed by the docker container because of OOM, jdk.internal.platform.Metrics.systemMetrics().getMemoryFailCount( ) interface has no chance to return 1, and the Java process returns exit code 137. The maximum heap is also 64M, The PR is changed to 1KB each time to ensure that the getMemoryFailCount() interface has a chance to return 1 and the test case has a chance to exit the for loop of memory allocation. ## test result before this PR: ![image](https://github.com/openjdk/jdk/assets/24123821/4554dd00-6da5-4529-907a-45e2df5c902b) ## test result after this PR: ![image](https://github.com/openjdk/jdk/assets/24123821/32ea4fc8-aa04-425e-8481-a920265d2b1f) - PR Comment: https://git.openjdk.org/jdk/pull/17514#issuecomment-1903589872
RFR: 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed
8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed - Commit messages: - 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed Changes: https://git.openjdk.org/jdk/pull/17514/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=17514&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8323640 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/17514.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17514/head:pull/17514 PR: https://git.openjdk.org/jdk/pull/17514
Withdrawn: 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed
On Fri, 12 Jan 2024 03:31:37 GMT, sendaoYan wrote: > 8323640: [TESTBUG]testMemoryFailCount in > jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because > OOM killed This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/17386
Re: Does anyone have context on jdk.httpserver?
On 22/01/2024 08:43, Robert Engels wrote: See github.com/robaho/httpserver for a more capable fork of the JDK code. Would love to create a PR to move the core changes back into the JDK but the net-dev folks don’t seem to be interested As I recall, it wasn't really possible to do any assessment because the "contribution" wasn't covered by the OCA. In addition, I think you said in one of the mails that it includes code from another project (for websocket or HTTP upgrade?) and it's a big deal to have to import and work through the license issues with 3rd party code. At a high level it seems reasonable to update the HTTP server implementation to work better with virtual threads but updating it to be a more fully featured HTTP server goes beyond what this HTTP server was/is intended for. -Alan
Re: RFR: 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed [v2]
> 8323640: [TESTBUG]testMemoryFailCount in > jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because > OOM killed sendaoYan 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: 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed Signed-off-by: sendaoYan - Changes: - all: https://git.openjdk.org/jdk/pull/17386/files - new: https://git.openjdk.org/jdk/pull/17386/files/e8a99fe4..9f0aa2a1 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17386&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17386&range=00-01 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/17386.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17386/head:pull/17386 PR: https://git.openjdk.org/jdk/pull/17386
Re: RFR: 8323717: Introduce test keyword for tests that need external dependencies
On Mon, 15 Jan 2024 10:48:23 GMT, Aleksey Shipilev wrote: > Some jtreg tests require resolvable external dependencies. This resolution is > delegated to JIB, which is not used in vanilla OpenJDK testing. It would be > convenient to add a keyword that marks tests that require these external > dependencies, so that we could exclude those tests from runs. This would > allow us to: a) run all tests in hotspot:tier4, which now excludes > `applications/` specifically; b) make all tests runs (#17422) cleaner on many > environments. > > I provisionally call this flag `external-dep`, but I am open for other > suggestions. > > Note that some tests that pull `@Artifact`-s provide special paths that do > limited testing anyway. However, there are tests which cannot run without > external dependencies at all. These include at least `applications/jcstress` > and `applications/scimark` tests. > > Ironically, I cannot run the jcstress test generator because the dependencies > are lacking here. I regenerated those test using a self-built jcstress 0.16 > bundle. > > Additional testing: > - [x] `make test TEST=applications/` fails > - [x] `JTREG_KEYWORDS=!external-dep make test TEST=applications/` passes, > skipping most of the tests Any takers? Maybe the audience should include core-libs too. - PR Comment: https://git.openjdk.org/jdk/pull/17421#issuecomment-1903486053