Re: RFR: 8333396: Performance regression of DecimalFormat.format [v8]
On Tue, 18 Jun 2024 10:00:33 GMT, lingjun-cg wrote:
>> ### Performance regression of DecimalFormat.format
>> From the output of perf, we can see the hottest regions contain atomic
>> instructions. But when run with JDK 11, there is no such problem. The
>> reason is the removed biased locking.
>> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
>> contains many synchronized methods.
>> So I added support for some new methods that accept StringBuilder which is
>> lock-free.
>>
>> ### Benchmark testcase
>>
>> @BenchmarkMode(Mode.AverageTime)
>> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
>> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
>> @State(Scope.Thread)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> public class JmhDecimalFormat {
>>
>> private DecimalFormat format;
>>
>> @Setup(Level.Trial)
>> public void setup() {
>> format = new DecimalFormat("#0.0");
>> }
>>
>> @Benchmark
>> public void testNewAndFormat() throws InterruptedException {
>> new DecimalFormat("#0.0").format(9524234.1236457);
>> }
>>
>> @Benchmark
>> public void testNewOnly() throws InterruptedException {
>> new DecimalFormat("#0.0");
>> }
>>
>> @Benchmark
>> public void testFormatOnly() throws InterruptedException {
>> format.format(9524234.1236457);
>> }
>> }
>>
>>
>> ### Test result
>> Current JDK before optimize
>>
>> Benchmark Mode CntScore Error Units
>> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
>> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
>> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>>
>>
>>
>> Current JDK after optimize
>>
>> Benchmark Mode CntScore Error Units
>> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
>> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
>> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>>
>>
>> ### JDK 11
>>
>> Benchmark Mode CntScore Error Units
>> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
>> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
>> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
>
> lingjun-cg has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 896: Performance regression of DecimalFormat.format
If change the visibility of `java.lang.AbstractStringBuilder` from
package-private to public, it seems more simple.
The `AbstractStringBuilder` is a sealed class, user code cannot extend it, and
it also used by other class like `String`.
-
PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2179655141
Re: RFR: 8333893: Optimization for StringBuilder append boolean & null [v8]
On Thu, 13 Jun 2024 09:51:21 GMT, Claes Redestad wrote: >> Shaojin Wen has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - rename benchmark >> - code format & use long address > > Replacing `StringUTF16.putChar` with an `Unsafe.putByte` construct seems safe > since the former already depends on bounds checks having been done elsewhere. > Replacing `byte[]` stores with `Unsafe.putByte` requires a thorough > examination as that would drop implicit bounds checks. > > On my M1 laptop I get similar numbers: > > Name Cnt Base Error Test Error Unit > Change > StringBuilders.appendWithBool8Latin1 15 7,500 ± 0,072 6,112 ± 0,049 ns/op > 1,23x (p = 0,000*) > StringBuilders.appendWithBool8Utf16 15 9,650 ± 0,021 7,304 ± 0,047 ns/op > 1,32x (p = 0,000*) > StringBuilders.appendWithNull8Latin1 15 7,118 ± 0,033 5,158 ± 0,062 ns/op > 1,38x (p = 0,000*) > StringBuilders.appendWithNull8Utf16 15 9,336 ± 0,083 6,870 ± 0,069 ns/op > 1,36x (p = 0,000*) > * = significant > > > On a linux-x64 workstation some of the micros regress, though: > > Name Cnt BaseErrorTest Error > Unit Change > StringBuilders.appendWithBool8Latin1 15 10.781 ± 0.236 16.279 ± 1.087 > ns/op 0.66x (p = 0.000*) > StringBuilders.appendWithBool8Utf16 15 22.942 ± 0.405 23.733 ± 0.792 > ns/op 0.97x (p = 0.001*) > StringBuilders.appendWithNull8Latin1 15 24.313 ± 10.479 24.397 ± 7.483 > ns/op 1.00x (p = 0.979 ) > StringBuilders.appendWithNull8Utf16 15 38.704 ± 5.972 27.542 ± 5.620 > ns/op 1.41x (p = 0.000*) > * = significant > > > `-XX:+TraceMergeStores` seem to indicate the merging happens as it should in > each case: > > > [TraceMergeStores]: Replace > 2493 StoreB === 1387 5370 2980 2939 [[ 1962 ]] @byte[int:>=0] > (java/lang/Cloneable,java/io/Serializable):exact+any *, idx=13; unsafe > Memory: @byte[int:>=0] > (java/lang/Cloneable,java/io/Serializable):NotNull:exact[2] *, idx=13; !jvms: > StringLatin1::putCharsAt @ bci:50 (line 834) > AbstractStringBuilder::appendNull @ bci:34 (line 642) > AbstractStringBuilder::append @ bci:5 (line 587) StringBuilder::append @ > bci:2 (line 179) StringBuilders::appendWithNull8Latin1 @ bci:38 (line 267) > StringBuilders_appendWithNull8Latin1_jmhTest::appendWithNull8Latin1_avgt_jmhStub > @ bci:17 (line 190) > 1962 StoreB === 1387 2493 2494 2442 [[ 1388 ]] @byte[int:>=0] > (java/lang/Cloneable,java/io/Serializable):exact+any *, idx=13; unsafe > Memory: @byte[int:>=0] > (java/lang/Cloneable,java/io/Serializable):NotNull:exact[2] *, idx=13; !jvms: > StringLatin1::putCharsAt @ bci:62 (line 835) > AbstractStringBuilder::appendNull @ bci:34 (line 642) > AbstractStringBuilder::append @ bci:5 (line 587) StringBuilder::append... The performance regression issue on x64 platform has been fixed. Please continue to review @cl4es @liach - PR Comment: https://git.openjdk.org/jdk/pull/19626#issuecomment-2179620100
Re: java.time lacks start and end aware period data types
Sorry to send as follow up, those "to summarize" sentence is a question, not a statement. Sorry for possible confusion On Thu, Jun 20, 2024, 00:12 Olexandr Rotan wrote: > I am aware of ThreeTen-Extra. The issue with this library is that it > lacks native transition to java.time types, java.time hierarchy is closed > and therefore there is no in-all-ways complete way to integrate it in a > project without seams all over the place. > > The issue with ranges and comparables is the fact that it is impossible to > implement multiple interfaces with different type parameters. This leads to > an issue, that, for example, Range cant be used against > BigInteger, Float, Long etc etc. This is why, I think, this idea in current > Java is inherently bad. > > So, to summarize, the main reason why there is no such types in java.time > is the fact that there just were tasks with higher priorities at the > moment. If so, I might do some research and draft some APIs myself and > present it to the Java team to determine whether it is ok or not to > continue development in this direction or not. > > > > On Wed, Jun 19, 2024 at 11:29 PM Stephen Colebourne > wrote: > >> There are a variety of ways that a range of the timeline can be defined. >> And some would argue that Java should have a general purpose Range class to >> cover all Comparable classes rather than a few specific ones for Java time. >> >> Ultimately, Java time chose to avoid the issues by not addressing the >> design space. There were plenty of other things to tackle at the time. >> >> BTW, see ThreeTen-Extra for some range and interval classes. >> >> Stephen >> >> >> >> On Wed, 19 Jun 2024, 20:44 Olexandr Rotan, >> wrote: >> >>> Greetings to the Java community. I have a question regarding the design >>> of java,time package. >>> >>> Commercial Java developers deal with time periods all the time with >>> different validations regarding intersection of periods, big data >>> processing, entity auditing etc etc. And it is surprising for everyone to >>> look into java.time for the first time and find out that there is no start >>> and end aware span data types at all! >>> >>> There is no rocket science in implementing such data types yourself, >>> that's for sure. However, as always, there are hidden spikes in these >>> waters.To use these types in JPA entities, you would have to write a custom >>> converter to familiar for database type, which ties project tightly to >>> database, which is contrary to what JPA was intended for . To use it in >>> auditing. one would have to write a custom auditor as no framework or >>> library knows about our custom data type. Same goes for big data >>> processing: no library or framework would support processing series of your >>> custom data type, at least unless you provide some adapter using library >>> mechanisms, if those exist at all. This rant could go on and on, but the >>> points are: >>> >>> 1) Lack of standardization for such data types leads to obstacles on >>> each step of development: constant writing of adapters, converters and blah >>> blah blah. Each of those serves as a space to make mistakes. One-two easy >>> implementations are not likely to be the source of errors, but the more a >>> project grows, the more dependencies it acquires, the more it is likely to >>> fall for some subtle specific feature of some library and spend hours or >>> even days debugging. >>> 2) One could rightfully argue that for small projects or microservice >>> projects with no shared codebase and different team for each service, this >>> whole mess I described above is overengineering. Seriously, not everyone >>> would want to go through all of that for one-two usages of class in the >>> domain layer of the project, and, moreover, not everyone should. However, >>> leaving it be "as is" is a screaming source of data inconsistencies, as it >>> is now responsibility of developer (or even worse, user) to keep in mind >>> the requirement to update all fields, that combined form that start- and >>> end-aware period (like start date/datetime and Period/Duration as common >>> way to emulate such thing) >>> >>> So, the question is: was it a conscious decision to omit such types back >>> in the day when java.time was designed, or is it a blindspot created by >>> deadlines/lack of resources/something else? >>> >>> Would appreciate anything some of you have to share on this topic. >>> >>> Best regards >>> >>
Integrated: 8309821: Link to hidden classes section in Class specification for Class::isHidden
On Wed, 19 Jun 2024 02:21:05 GMT, Joe Darcy wrote: > Simple doc improvement. This pull request has now been integrated. Changeset: 4e58d8c8 Author:Joe Darcy URL: https://git.openjdk.org/jdk/commit/4e58d8c897d845cfa73780264481da174d46acb4 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod 8309821: Link to hidden classes section in Class specification for Class::isHidden Reviewed-by: iris, rriggs - PR: https://git.openjdk.org/jdk/pull/19781
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v3]
On Wed, 19 Jun 2024 21:22:10 GMT, Maurizio Cimadamore wrote: >> Jorn Vernee has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - review comments >> - add man page > > src/jdk.jdeps/share/man/jnativescan.1 line 68: > >> 66: .TP >> 67: \f[V]--class-path\f[R] \f[I]path\f[R] >> 68: Used to specify a path list of class path jar files to be scanned. > > I can't parse "path list of class path jar files". What is the argument here? > I think it has to be a list of jars? If so, one way could be `a list of > paths pointing to the jar files to be scanned` My suggestion is to separate up the "what is this argument?" (is a list of paths, to jar files), from the "semantics" (e.g. the classes in the jarfiles are assumed to belong to the unnamed module). The you can mirror this structure in the module option. - PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646700042
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v3]
On Wed, 19 Jun 2024 21:16:46 GMT, Jorn Vernee wrote: >> This PR adds a new JDK tool, called `jnativescan`, that can be used to find >> code that accesses native functionality. Currently this includes `native` >> method declarations, and methods marked with `@Restricted`. >> >> The tool accepts a list of class path and module path entries through >> `--class-path` and `--module-path`, and a set of root modules through >> `--add-modules`, as well as an optional target release with `--release`. >> >> The default mode is for the tool to report all uses of `@Restricted` >> methods, and `native` method declaration in a tree-like structure: >> >> >> app.jar (ALL-UNNAMED): >> main.Main: >> main.Main::main(String[])void references restricted methods: >> java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment >> main.Main::m()void is a native method declaration >> >> >> The `--print-native-access` option can be used print out all the module >> names of modules doing native access in a comma separated list. For class >> path code, this will print out `ALL-UNNAMED`. >> >> Testing: >> - `langtools_jnativescan` tests. >> - Running the tool over jextract's libclang bindings, which use the FFM API, >> and thus has a lot of references to `@Restricted` methods. >> - tier 1-3 > > Jorn Vernee has updated the pull request incrementally with two additional > commits since the last revision: > > - review comments > - add man page Nice changes! src/jdk.jdeps/share/man/jnativescan.1 line 43: > 41: .PP > 42: jnativescan - static analysis tool that scans one or more jar files for > 43: uses of native functionality, such as restricted methods or I think using `functionalities` would be better. Also "restricted method **calls** works better too (so it's restricted method **calls** and native method **declarations. src/jdk.jdeps/share/man/jnativescan.1 line 55: > 53: The \f[V]jnative\f[R] tool is a static analysis tool provided by the JDK > 54: that scans a JAR file for uses of native functionality, such as > 55: restricted methods or \f[V]native\f[R] method declarations. Same comments as above src/jdk.jdeps/share/man/jnativescan.1 line 60: > 58: configuration, as well as a set of root modules, and a target release. > 59: It scans the jars on the class and module paths, and reports uses of > 60: native functionality either in a tree like structure, which also functionalities src/jdk.jdeps/share/man/jnativescan.1 line 68: > 66: .TP > 67: \f[V]--class-path\f[R] \f[I]path\f[R] > 68: Used to specify a path list of class path jar files to be scanned. I can't parse "path list of class path jar files". What is the argument here? I think it has to be a list of jars? If so, one way could be `a list of paths pointing to the jar files to be scanned` src/jdk.jdeps/share/man/jnativescan.1 line 76: > 74: .TP > 75: \f[V]--module-path\f[R] \f[I]path\f[R] > 76: Used to specify a path list of jar files or directories containing This is also hard to read. Perhaps `a list of paths pointing to the jar files to be scanned, or the directories containing said jar files.` src/jdk.jdeps/share/man/jnativescan.1 line 112: > 110: Used to specifiy a comma-separated list of module names that indicate > 111: the root modules to scan. > 112: All the modules in the root set will be scanned, as well as any modules Suggestion: All the root modules will be scanned, as well as any modules src/jdk.jdeps/share/man/jnativescan.1 line 121: > 119: .TP > 120: \f[V]--release\f[R] \f[I]version\f[R] > 121: Used to specify the Java SE release that specifies the set of restricted In principle, the release could also affect which methods are native or not? src/jdk.jdeps/share/man/jnativescan.1 line 127: > 125: This option should be set to the version of the runtime under which the > 126: application is eventually intended to be run. > 127: If this flag is omitted, the version of \f[V]jnativescan\f[R] is used as Maybe we should point to `Runtime.version()` instead? src/jdk.jdeps/share/man/jnativescan.1 line 146: > 144: For the class path, the tool will scan all jar files, including those > 145: found recursively through the \f[V]Class-Path\f[R] manifest attribute. > 146: For the module path, the tool scans all modules in the root module set Note: sometimes you say root modules, sometimes root module set. I'd suggest to pick one and remain consistent. src/jdk.jdeps/share/man/jnativescan.1 line 166: > 164: .fi > 165: .PP > 166: \f[V]app.jar (ALL-UNNAMED)\f[R] is the path to the jar file, with the This is a good explanation, I'm not sure whether it's necessary, as the output seems quite self-explanatory. But I'm ok either way. - Marked as reviewed by mcimadamore (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19774#pullrequestreview-2129052320 PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646696108 PR Review Comment:
Re: RFR: 8307818: Convert Indify tool to Classfile API [v11]
On Wed, 19 Jun 2024 12:04:27 GMT, Oussama Louati wrote:
>> An indify tool in j.l.i tests (also in vmTestBase) convert some source-code
>> private static methods with MT_ MH_, and INDY_ prefixes into MethodHandle,
>> MethodType, and CallSite constants.
>> It currently uses ad-hoc code to process class files and intends to migrate
>> to ASM; but since we have the Classfile API, we can migrate to Classfile API
>> instead.
>
> Oussama Louati has updated the pull request incrementally with one additional
> commit since the last revision:
>
> fix: fix comments
Formatting nits:
More formatting nits:
test/jdk/java/lang/invoke/indify/Indify.java line 656:
> 654: for(PoolEntry entry : classModel.constantPool()){
> 655: char mark = 0;
> 656: if(poolMarks[entry.index()] != 0) continue;
Suggestion:
boolean initializeMarks() {
boolean anyMarkChanged = false;
for (PoolEntry entry : classModel.constantPool()) {
char mark = 0;
if (poolMarks[entry.index()] != 0) continue;
test/jdk/java/lang/invoke/indify/Indify.java line 674:
> 672: if(poolMarks[memberRefEntry.owner().index()] != 0){
> 673: mark = poolMarks[memberRefEntry.owner().index()];
> 674: }
Suggestion:
if (entry instanceof Utf8Entry utf8Entry) {
mark = nameMark(utf8Entry.stringValue());
}
if (entry instanceof ClassEntry classEntry) {
mark = nameMark(classEntry.asInternalName());
}
if (entry instanceof StringEntry stringEntry) {
mark = nameMark(stringEntry.stringValue());
}
if (entry instanceof NameAndTypeEntry nameAndTypeEntry) {
mark = nameAndTypeMark(nameAndTypeEntry.name(),
nameAndTypeEntry.type());
}
if (entry instanceof MemberRefEntry memberRefEntry) {
poolMarks[memberRefEntry.owner().index()] =
nameMark(memberRefEntry.owner().asInternalName());
if(poolMarks[memberRefEntry.owner().index()] != 0){
mark = poolMarks[memberRefEntry.owner().index()];
}
test/jdk/java/lang/invoke/indify/Indify.java line 678:
> 676:
> if(memberRefEntry.owner().equals(classModel.thisClass())){
> 677: mark =
> nameMark(memberRefEntry.name().stringValue());
> 678: }
Suggestion:
else {
if
(memberRefEntry.owner().equals(classModel.thisClass())) {
mark =
nameMark(memberRefEntry.name().stringValue());
}
test/jdk/java/lang/invoke/indify/Indify.java line 686:
> 684: return anyMarkChanged;
> 685: }
> 686: char nameAndTypeMark(Utf8Entry name, Utf8Entry type){
Suggestion:
}
char nameAndTypeMark(Utf8Entry name, Utf8Entry type){
test/jdk/java/lang/invoke/indify/Indify.java line 691:
> 689: String descriptor = type.stringValue();
> 690: String requiredType;
> 691: switch (mark){
Suggestion:
switch (mark) {
test/jdk/java/lang/invoke/indify/Indify.java line 696:
> 694: default: return 0;
> 695: }
> 696: if(matchType(descriptor, requiredType)) return mark;
Suggestion:
if (matchType(descriptor, requiredType)) return mark;
-
PR Review: https://git.openjdk.org/jdk/pull/18841#pullrequestreview-2129054394
PR Review: https://git.openjdk.org/jdk/pull/18841#pullrequestreview-2129056939
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1646696601
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1646697107
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1646697252
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1646698327
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1646698374
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1646698432
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v3]
On Wed, 19 Jun 2024 18:57:43 GMT, Jorn Vernee wrote: >> I can do that, but I think this will always be a bit awkward since these >> types don't have a common super type that exposes the needed information. > >> these types don't have a common super type that exposes the needed >> information > > No wait, they actually do :) That's just `MemberRefEntry`. I was able to eliminate this method entirely, now the caller just creates a `MethodRef` and passes that to the other `isRestrictedMethod` overload. - PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646691228
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v3]
On Wed, 19 Jun 2024 21:13:33 GMT, Jorn Vernee wrote:
>> This PR adds a new JDK tool, called `jnativescan`, that can be used to find
>> code that accesses native functionality. Currently this includes `native`
>> method declarations, and methods marked with `@Restricted`.
>>
>> The tool accepts a list of class path and module path entries through
>> `--class-path` and `--module-path`, and a set of root modules through
>> `--add-modules`, as well as an optional target release with `--release`.
>>
>> The default mode is for the tool to report all uses of `@Restricted`
>> methods, and `native` method declaration in a tree-like structure:
>>
>>
>> app.jar (ALL-UNNAMED):
>> main.Main:
>> main.Main::main(String[])void references restricted methods:
>> java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment
>> main.Main::m()void is a native method declaration
>>
>>
>> The `--print-native-access` option can be used print out all the module
>> names of modules doing native access in a comma separated list. For class
>> path code, this will print out `ALL-UNNAMED`.
>>
>> Testing:
>> - `langtools_jnativescan` tests.
>> - Running the tool over jextract's libclang bindings, which use the FFM API,
>> and thus has a lot of references to `@Restricted` methods.
>> - tier 1-3
>
> Jorn Vernee has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - review comments
> - add man page
src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/NativeMethodFinder.java
line 89:
> 87: throw new JNativeScanFatalError("Error while
> processing method: " +
> 88: MethodRef.ofModel(methodModel), e);
> 89: }
I'm re-wrapping the exception here, thrown from `SystemClassResolver::lookup`,
in order to include the method that called the missing system class in the
error message as well. (See also the code added in `Main` to print out an
exception cause if it is a `JNativeScanFatalError`).
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646689741
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v3]
On Wed, 19 Jun 2024 19:59:19 GMT, Jorn Vernee wrote: >> src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/JNativeScanTask.java >> line 76: >> >>> 74: URI location = m.reference().location().orElseThrow(); >>> 75: Path path = Path.of(location); >>> 76: checkRegularJar(path); >> >> if exploded modules aren't supported (as discussed offline), maybe the error >> message generated here should be more specific? > > What do you suggest? Just a note in the error message that exploded > modules/class paths are not supported? Something like that yes - PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646692395
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v3]
On Wed, 19 Jun 2024 18:02:08 GMT, Jorn Vernee wrote:
>> src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/ClassResolver.java
>> line 126:
>>
>>> 124:
>>> 125: private static Map packageToSystemModule() {
>>> 126: List descriptors =
>>> ModuleFinder.ofSystem()
>>
>> Is it a problem that we compute the package -> module map using the runtime
>> info (so latest version) but then all the other info is taken from a
>> release-specific symbol file? E.g. say that package "foo" was moved from
>> module "A" to module "B" in version N, and that user passes N - 1 as release
>> to the scan tool - would that work?
>
> That's a good point, I don't think that scenario will work. We should really
> use the release specific info if we can. I think that's relatively easy to
> do, will take a look.
Ok, I managed to implement this, but I don't think we can actually test this
use case, since (AFAIK) there's never been a case of a package being moved to a
different module under the same name.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646688656
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v2]
On Wed, 19 Jun 2024 19:00:22 GMT, Jorn Vernee wrote: >> This PR adds a new JDK tool, called `jnativescan`, that can be used to find >> code that accesses native functionality. Currently this includes `native` >> method declarations, and methods marked with `@Restricted`. >> >> The tool accepts a list of class path and module path entries through >> `--class-path` and `--module-path`, and a set of root modules through >> `--add-modules`, as well as an optional target release with `--release`. >> >> The default mode is for the tool to report all uses of `@Restricted` >> methods, and `native` method declaration in a tree-like structure: >> >> >> app.jar (ALL-UNNAMED): >> main.Main: >> main.Main::main(String[])void references restricted methods: >> java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment >> main.Main::m()void is a native method declaration >> >> >> The `--print-native-access` option can be used print out all the module >> names of modules doing native access in a comma separated list. For class >> path code, this will print out `ALL-UNNAMED`. >> >> Testing: >> - `langtools_jnativescan` tests. >> - Running the tool over jextract's libclang bindings, which use the FFM API, >> and thus has a lot of references to `@Restricted` methods. >> - tier 1-3 > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > Update src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/Main.java > > Co-authored-by: Maurizio Cimadamore > <54672762+mcimadam...@users.noreply.github.com> I've also added a first version of the man page for the tool. I'm not every familiar with the JDK man pages, as I rarely use them myself, so there's probably things missing. Please let me know. - PR Comment: https://git.openjdk.org/jdk/pull/19774#issuecomment-2179460047
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v3]
> This PR adds a new JDK tool, called `jnativescan`, that can be used to find > code that accesses native functionality. Currently this includes `native` > method declarations, and methods marked with `@Restricted`. > > The tool accepts a list of class path and module path entries through > `--class-path` and `--module-path`, and a set of root modules through > `--add-modules`, as well as an optional target release with `--release`. > > The default mode is for the tool to report all uses of `@Restricted` methods, > and `native` method declaration in a tree-like structure: > > > app.jar (ALL-UNNAMED): > main.Main: > main.Main::main(String[])void references restricted methods: > java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment > main.Main::m()void is a native method declaration > > > The `--print-native-access` option can be used print out all the module names > of modules doing native access in a comma separated list. For class path > code, this will print out `ALL-UNNAMED`. > > Testing: > - `langtools_jnativescan` tests. > - Running the tool over jextract's libclang bindings, which use the FFM API, > and thus has a lot of references to `@Restricted` methods. > - tier 1-3 Jorn Vernee has updated the pull request incrementally with two additional commits since the last revision: - review comments - add man page - Changes: - all: https://git.openjdk.org/jdk/pull/19774/files - new: https://git.openjdk.org/jdk/pull/19774/files/861965b7..b5468440 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19774=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19774=01-02 Stats: 645 lines in 10 files changed: 467 ins; 155 del; 23 mod Patch: https://git.openjdk.org/jdk/pull/19774.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19774/head:pull/19774 PR: https://git.openjdk.org/jdk/pull/19774
Re: java.time lacks start and end aware period data types
I am aware of ThreeTen-Extra. The issue with this library is that it lacks native transition to java.time types, java.time hierarchy is closed and therefore there is no in-all-ways complete way to integrate it in a project without seams all over the place. The issue with ranges and comparables is the fact that it is impossible to implement multiple interfaces with different type parameters. This leads to an issue, that, for example, Range cant be used against BigInteger, Float, Long etc etc. This is why, I think, this idea in current Java is inherently bad. So, to summarize, the main reason why there is no such types in java.time is the fact that there just were tasks with higher priorities at the moment. If so, I might do some research and draft some APIs myself and present it to the Java team to determine whether it is ok or not to continue development in this direction or not. On Wed, Jun 19, 2024 at 11:29 PM Stephen Colebourne wrote: > There are a variety of ways that a range of the timeline can be defined. > And some would argue that Java should have a general purpose Range class to > cover all Comparable classes rather than a few specific ones for Java time. > > Ultimately, Java time chose to avoid the issues by not addressing the > design space. There were plenty of other things to tackle at the time. > > BTW, see ThreeTen-Extra for some range and interval classes. > > Stephen > > > > On Wed, 19 Jun 2024, 20:44 Olexandr Rotan, > wrote: > >> Greetings to the Java community. I have a question regarding the design >> of java,time package. >> >> Commercial Java developers deal with time periods all the time with >> different validations regarding intersection of periods, big data >> processing, entity auditing etc etc. And it is surprising for everyone to >> look into java.time for the first time and find out that there is no start >> and end aware span data types at all! >> >> There is no rocket science in implementing such data types yourself, >> that's for sure. However, as always, there are hidden spikes in these >> waters.To use these types in JPA entities, you would have to write a custom >> converter to familiar for database type, which ties project tightly to >> database, which is contrary to what JPA was intended for . To use it in >> auditing. one would have to write a custom auditor as no framework or >> library knows about our custom data type. Same goes for big data >> processing: no library or framework would support processing series of your >> custom data type, at least unless you provide some adapter using library >> mechanisms, if those exist at all. This rant could go on and on, but the >> points are: >> >> 1) Lack of standardization for such data types leads to obstacles on each >> step of development: constant writing of adapters, converters and blah blah >> blah. Each of those serves as a space to make mistakes. One-two easy >> implementations are not likely to be the source of errors, but the more a >> project grows, the more dependencies it acquires, the more it is likely to >> fall for some subtle specific feature of some library and spend hours or >> even days debugging. >> 2) One could rightfully argue that for small projects or microservice >> projects with no shared codebase and different team for each service, this >> whole mess I described above is overengineering. Seriously, not everyone >> would want to go through all of that for one-two usages of class in the >> domain layer of the project, and, moreover, not everyone should. However, >> leaving it be "as is" is a screaming source of data inconsistencies, as it >> is now responsibility of developer (or even worse, user) to keep in mind >> the requirement to update all fields, that combined form that start- and >> end-aware period (like start date/datetime and Period/Duration as common >> way to emulate such thing) >> >> So, the question is: was it a conscious decision to omit such types back >> in the day when java.time was designed, or is it a blindspot created by >> deadlines/lack of resources/something else? >> >> Would appreciate anything some of you have to share on this topic. >> >> Best regards >> >
Re: java.time lacks start and end aware period data types
There are a variety of ways that a range of the timeline can be defined. And some would argue that Java should have a general purpose Range class to cover all Comparable classes rather than a few specific ones for Java time. Ultimately, Java time chose to avoid the issues by not addressing the design space. There were plenty of other things to tackle at the time. BTW, see ThreeTen-Extra for some range and interval classes. Stephen On Wed, 19 Jun 2024, 20:44 Olexandr Rotan, wrote: > Greetings to the Java community. I have a question regarding the design of > java,time package. > > Commercial Java developers deal with time periods all the time with > different validations regarding intersection of periods, big data > processing, entity auditing etc etc. And it is surprising for everyone to > look into java.time for the first time and find out that there is no start > and end aware span data types at all! > > There is no rocket science in implementing such data types yourself, > that's for sure. However, as always, there are hidden spikes in these > waters.To use these types in JPA entities, you would have to write a custom > converter to familiar for database type, which ties project tightly to > database, which is contrary to what JPA was intended for . To use it in > auditing. one would have to write a custom auditor as no framework or > library knows about our custom data type. Same goes for big data > processing: no library or framework would support processing series of your > custom data type, at least unless you provide some adapter using library > mechanisms, if those exist at all. This rant could go on and on, but the > points are: > > 1) Lack of standardization for such data types leads to obstacles on each > step of development: constant writing of adapters, converters and blah blah > blah. Each of those serves as a space to make mistakes. One-two easy > implementations are not likely to be the source of errors, but the more a > project grows, the more dependencies it acquires, the more it is likely to > fall for some subtle specific feature of some library and spend hours or > even days debugging. > 2) One could rightfully argue that for small projects or microservice > projects with no shared codebase and different team for each service, this > whole mess I described above is overengineering. Seriously, not everyone > would want to go through all of that for one-two usages of class in the > domain layer of the project, and, moreover, not everyone should. However, > leaving it be "as is" is a screaming source of data inconsistencies, as it > is now responsibility of developer (or even worse, user) to keep in mind > the requirement to update all fields, that combined form that start- and > end-aware period (like start date/datetime and Period/Duration as common > way to emulate such thing) > > So, the question is: was it a conscious decision to omit such types back > in the day when java.time was designed, or is it a blindspot created by > deadlines/lack of resources/something else? > > Would appreciate anything some of you have to share on this topic. > > Best regards >
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v2]
On Wed, 19 Jun 2024 17:41:36 GMT, Maurizio Cimadamore wrote: >> Jorn Vernee has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Update src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/Main.java >> >> Co-authored-by: Maurizio Cimadamore >> <54672762+mcimadam...@users.noreply.github.com> > > src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/JNativeScanTask.java > line 76: > >> 74: URI location = m.reference().location().orElseThrow(); >> 75: Path path = Path.of(location); >> 76: checkRegularJar(path); > > if exploded modules aren't supported (as discussed offline), maybe the error > message generated here should be more specific? What do you suggest? Just a note in the error message that exploded modules/class paths are not supported? - PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646643125
java.time lacks start and end aware period data types
Greetings to the Java community. I have a question regarding the design of java,time package. Commercial Java developers deal with time periods all the time with different validations regarding intersection of periods, big data processing, entity auditing etc etc. And it is surprising for everyone to look into java.time for the first time and find out that there is no start and end aware span data types at all! There is no rocket science in implementing such data types yourself, that's for sure. However, as always, there are hidden spikes in these waters.To use these types in JPA entities, you would have to write a custom converter to familiar for database type, which ties project tightly to database, which is contrary to what JPA was intended for . To use it in auditing. one would have to write a custom auditor as no framework or library knows about our custom data type. Same goes for big data processing: no library or framework would support processing series of your custom data type, at least unless you provide some adapter using library mechanisms, if those exist at all. This rant could go on and on, but the points are: 1) Lack of standardization for such data types leads to obstacles on each step of development: constant writing of adapters, converters and blah blah blah. Each of those serves as a space to make mistakes. One-two easy implementations are not likely to be the source of errors, but the more a project grows, the more dependencies it acquires, the more it is likely to fall for some subtle specific feature of some library and spend hours or even days debugging. 2) One could rightfully argue that for small projects or microservice projects with no shared codebase and different team for each service, this whole mess I described above is overengineering. Seriously, not everyone would want to go through all of that for one-two usages of class in the domain layer of the project, and, moreover, not everyone should. However, leaving it be "as is" is a screaming source of data inconsistencies, as it is now responsibility of developer (or even worse, user) to keep in mind the requirement to update all fields, that combined form that start- and end-aware period (like start date/datetime and Period/Duration as common way to emulate such thing) So, the question is: was it a conscious decision to omit such types back in the day when java.time was designed, or is it a blindspot created by deadlines/lack of resources/something else? Would appreciate anything some of you have to share on this topic. Best regards
Re: RFR: 8322420: [Linux] cgroup v2: Limits in parent nested control groups are not detected [v11]
On Wed, 22 May 2024 15:02:18 GMT, Jan Kratochvil wrote: >> The testcase requires root permissions. >> >> Designed by Severin Gehwolf, implemented by Jan Kratochvil. > > Jan Kratochvil has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 74 commits: > > - . > - . > - Merge branch 'jdk-8331560-cgroup-controller-delegation-merge-diamond' into > jdk-8331560-cgroup-controller-delegation-merge-cgroup > - diamond > - Merge branch 'jerboaarefactor-merge-cgroup' into > jdk-8331560-cgroup-controller-delegation-merge-cgroup > - Merge branch 'jerboaarefactor-merge' into jerboaarefactor-merge-cgroup > - Merge branch 'master' into jerboaarefactor-merge > - whitespace fix > - centos7 compat > - 64a5feb6: > - ... and 64 more: https://git.openjdk.org/jdk/compare/3d511ff6...25c0287d Keep open. - PR Comment: https://git.openjdk.org/jdk/pull/17198#issuecomment-2179330671
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v2]
On Wed, 19 Jun 2024 17:45:14 GMT, Jorn Vernee wrote:
>> src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/RestrictedMethodFinder.java
>> line 120:
>>
>>> 118: Optional info =
>>> systemClassResolver.lookup(methodRef.owner());
>>> 119: if (!info.isPresent()) {
>>> 120: return false;
>>
>> Is this just `false` or maybe a warning that a certain owner could not be
>> resolved (maybe if running with enough debug options) ?
>
> Yes, thought about that yesterday as well. Good catch
Thinking a bit more: we also use the optional being empty to know if the owner
is not a system class, so I think this code here is what we want. We can
however throw an exception if a class is not found in
`SystemClassResolver::lookup`. Since, if a class is inside a system module
package, it should be found as well (unless the user sets the release to the
wrong version).
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646606901
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v2]
On Wed, 19 Jun 2024 18:09:15 GMT, Jorn Vernee wrote: > these types don't have a common super type that exposes the needed information No wait, they actually do :) That's just `MemberRefEntry`. - PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646604479
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v2]
> This PR adds a new JDK tool, called `jnativescan`, that can be used to find > code that accesses native functionality. Currently this includes `native` > method declarations, and methods marked with `@Restricted`. > > The tool accepts a list of class path and module path entries through > `--class-path` and `--module-path`, and a set of root modules through > `--add-modules`, as well as an optional target release with `--release`. > > The default mode is for the tool to report all uses of `@Restricted` methods, > and `native` method declaration in a tree-like structure: > > > app.jar (ALL-UNNAMED): > main.Main: > main.Main::main(String[])void references restricted methods: > java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment > main.Main::m()void is a native method declaration > > > The `--print-native-access` option can be used print out all the module names > of modules doing native access in a comma separated list. For class path > code, this will print out `ALL-UNNAMED`. > > Testing: > - `langtools_jnativescan` tests. > - Running the tool over jextract's libclang bindings, which use the FFM API, > and thus has a lot of references to `@Restricted` methods. > - tier 1-3 Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: Update src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/Main.java Co-authored-by: Maurizio Cimadamore <54672762+mcimadam...@users.noreply.github.com> - Changes: - all: https://git.openjdk.org/jdk/pull/19774/files - new: https://git.openjdk.org/jdk/pull/19774/files/9e8dad03..861965b7 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19774=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19774=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19774.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19774/head:pull/19774 PR: https://git.openjdk.org/jdk/pull/19774
Re: RFR: 8266431: Dual-Pivot Quicksort improvements (Radix sort) [v11]
On Fri, 10 May 2024 22:08:47 GMT, Srinivas Vamsi Parasa wrote: >> Hello Vamsi (@vamsi-parasa), >> >> Could you please run the new benchmarking to finalize the best version? >> What you need is to compile and run JavaBenchmarkHarness: >> >> javac --patch-module java.base=. -d classes *.java >> java -XX:CompileThreshold=1 -XX:-TieredCompilation --patch-module >> java.base=classes -cp classes java.util.JavaBenchmarkHarness >> >> Find the sources there: >> >> https://github.com/iaroslavski/sorting/blob/master/radixsort/DualPivotQuicksort_b01.java >> https://github.com/iaroslavski/sorting/blob/master/radixsort/DualPivotQuicksort_r31_11.java >> https://github.com/iaroslavski/sorting/blob/master/radixsort/DualPivotQuicksort_r31_11a.java >> https://github.com/iaroslavski/sorting/blob/master/radixsort/DualPivotQuicksort_r31_12.java >> https://github.com/iaroslavski/sorting/blob/master/radixsort/DualPivotQuicksort_r31_12a.java >> https://github.com/iaroslavski/sorting/blob/master/radixsort/JavaBenchmarkHarness.java >> >> Thank you, >> Vladimir > > Hi Vladimir (@iaroslavski), > > Please see the data below. > > Thanks, > Vamsi > > > name | builder | size | mode | count | score > -- | -- | -- | -- | -- | -- > b01 | RANDOM | 600 | avg | 325677 | 6.861 > b01 | RANDOM | 3000 | avg | 52041 | 77.313 > b01 | RANDOM | 9 | avg | 1217 | 4315.41 > b01 | RANDOM | 40 | avg | 242 | 22110.95 > b01 | RANDOM | 100 | avg | 90 | 58613.45 > b01 | REPEATED | 600 | avg | 651354 | 1.993 > b01 | REPEATED | 3000 | avg | 104083 | 13.026 > b01 | REPEATED | 9 | avg | 2435 | 741.97 > b01 | REPEATED | 40 | avg | 484 | 3161.073 > b01 | REPEATED | 100 | avg | 180 | 8363.671 > b01 | STAGGER | 600 | avg | 1954062 | 9.124 > b01 | STAGGER | 3000 | avg | 312251 | 10.026 > b01 | STAGGER | 9 | avg | 7305 | 286.313 > b01 | STAGGER | 40 | avg | 1453 | 1278.758 > b01 | STAGGER | 100 | avg | 542 | 3242.849 > b01 | SHUFFLE | 600 | avg | 325677 | 5.113 > b01 | SHUFFLE | 3000 | avg | 52041 | 28.85 > b01 | SHUFFLE | 9 | avg | 1217 | 1368.91 > b01 | SHUFFLE | 40 | avg | 242 | 5718.052 > b01 | SHUFFLE | 100 | avg | 90 | 15376.1 > r31_11 | RANDOM | 600 | avg | 325677 | 4.305 > r31_11 | RANDOM | 3000 | avg | 52041 | 73.399 > r31_11 | RANDOM | 9 | avg | 1217 | 3963.515 > r31_11 | RANDOM | 40 | avg | 242 | 19841.07 > r31_11 | RANDOM | 100 | avg | 90 | 53372.63 > r31_11 | REPEATED | 600 | avg | 651354 | 1.208 > r31_11 | REPEATED | 3000 | avg | 104083 | 6.206 > r31_11 | REPEATED | 9 | avg | 2435 | 614.159 > r31_11 | REPEATED | 40 | avg | 484 | 2615.923 > r31_11 | REPEATED | 100 | avg | 180 | 6553.801 > r31_11 | STAGGER | 600 | avg | 1954062 | 1.023 > r31_11 | STAGGER | 3000 | avg | 312251 | 4.406 > r31_11 | STAGGER | 9 | avg | 7305 | 129.76 > r31_11 | STAGGER | 40 | avg | 1453 | 600.581 > r31_11 | STAGGER | 100 | avg | 542 | 1588.827 > r31_11 | SHUFFLE | 600 | avg | 325677 | 2.887 > r31_11 | SHUFFLE | 3000 | avg | 52041 | 17.901 > r31_11 | SHUFFLE | 9 | avg | 1217 | 1033.808 > r31_11 | SHUFFLE | 40 | avg | 242 | 4686.188 > r31_11 | SHUFFLE | 100 | avg | 90 | 11589.67 > r31_11a | RANDOM | 600 | avg | 325677 | 4.277 > r31_11a | RANDOM | 3000 | avg | 52041 | 70.578 > r31_11a | RANDOM | 9 | avg | 1217 | 3963.836 > r31_11a | RANDOM | 40 | avg | 242 | 19771.67 > r31_11a | RANDOM | 100 | avg | 90 | 53417.95 > r31_11a | REPEATED | 600 | avg | 651354 | 1.112 > r31_11a | REPEATED | 3000 | avg | 104083 | 5.662 > r31_11a | REPEATED | 9 | avg | 2435 | 596.963 > r31_11a | REPEATED | 40 | avg | 484 | 2618.076 > r31_11a | REPEATED | 100 | avg | 180 | 6597.833 > r31_1... Hello Vamsi (@vamsi-parasa), Could you please run the new benchmarking to check sorting of all data types? What you need is to compile and run JavaBenchmarkHarness: javac --patch-module java.base=. -d classes *.java java -XX:CompileThreshold=1 -XX:-TieredCompilation --patch-module java.base=classes -cp classes java.util.JavaBenchmarkHarness Find the sources there: https://github.com/iaroslavski/sorting/blob/master/radixsort/DualPivotQuicksort_b01.java https://github.com/iaroslavski/sorting/blob/master/radixsort/DualPivotQuicksort_r32.java https://github.com/iaroslavski/sorting/blob/master/radixsort/JavaBenchmarkHarness.java Thank you, Vladimir - PR Comment: https://git.openjdk.org/jdk/pull/13568#issuecomment-2179307223
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods
On Wed, 19 Jun 2024 18:00:37 GMT, Jorn Vernee wrote: >> src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/JNativeScanTask.java >> line 81: >> >>> 79: >>> 80: Map>> >>> allRestrictedMethods; >>> 81: try(ClassResolver classesToScan = >>> ClassResolver.forScannedModules(modulesToScan, version); >> >> Could it make sense here to "chain" the two resolvers into one, and just >> pass the chained one to the restricted finder? > > We need to scan all the classes in the `classesToScan` resolver, but only > look for restricted methods in the system resolver. So, they need to stay > separate. Ah, ok! - PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646584528
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods
On Wed, 19 Jun 2024 17:28:23 GMT, Maurizio Cimadamore
wrote:
>> This PR adds a new JDK tool, called `jnativescan`, that can be used to find
>> code that accesses native functionality. Currently this includes `native`
>> method declarations, and methods marked with `@Restricted`.
>>
>> The tool accepts a list of class path and module path entries through
>> `--class-path` and `--module-path`, and a set of root modules through
>> `--add-modules`, as well as an optional target release with `--release`.
>>
>> The default mode is for the tool to report all uses of `@Restricted`
>> methods, and `native` method declaration in a tree-like structure:
>>
>>
>> app.jar (ALL-UNNAMED):
>> main.Main:
>> main.Main::main(String[])void references restricted methods:
>> java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment
>> main.Main::m()void is a native method declaration
>>
>>
>> The `--print-native-access` option can be used print out all the module
>> names of modules doing native access in a comma separated list. For class
>> path code, this will print out `ALL-UNNAMED`.
>>
>> Testing:
>> - `langtools_jnativescan` tests.
>> - Running the tool over jextract's libclang bindings, which use the FFM API,
>> and thus has a lot of references to `@Restricted` methods.
>> - tier 1-3
>
> src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/RestrictedMethodFinder.java
> line 105:
>
>> 103: return switch (method) {
>> 104: case MethodRefEntry mre ->
>> 105: isRestrictedMethod(mre.owner().asSymbol(),
>> mre.name().stringValue(), mre.typeSymbol());
>
> Do we really need these two identical calls to `isRestrictedMethod` ? Note
> that both `MethodRefEntry` and `InterfaceMethodEntry` are subclasses of
> `MemberRefEntry`, so perhaps the other `isRestrictedMethod` can just take
> `MemberRefEntry`?
>
> Or maybe we can have different factories in `MethodRef` one that takes
> `InterfaceMethodEntry` and another that takes `MemberRefEntry`.
I can do that, but I think this will always be a bit awkward since these types
don't have a common super type that exposes the needed information.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646571620
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods
On Wed, 19 Jun 2024 17:53:12 GMT, Maurizio Cimadamore
wrote:
>> This PR adds a new JDK tool, called `jnativescan`, that can be used to find
>> code that accesses native functionality. Currently this includes `native`
>> method declarations, and methods marked with `@Restricted`.
>>
>> The tool accepts a list of class path and module path entries through
>> `--class-path` and `--module-path`, and a set of root modules through
>> `--add-modules`, as well as an optional target release with `--release`.
>>
>> The default mode is for the tool to report all uses of `@Restricted`
>> methods, and `native` method declaration in a tree-like structure:
>>
>>
>> app.jar (ALL-UNNAMED):
>> main.Main:
>> main.Main::main(String[])void references restricted methods:
>> java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment
>> main.Main::m()void is a native method declaration
>>
>>
>> The `--print-native-access` option can be used print out all the module
>> names of modules doing native access in a comma separated list. For class
>> path code, this will print out `ALL-UNNAMED`.
>>
>> Testing:
>> - `langtools_jnativescan` tests.
>> - Running the tool over jextract's libclang bindings, which use the FFM API,
>> and thus has a lot of references to `@Restricted` methods.
>> - tier 1-3
>
> src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/ClassResolver.java line
> 126:
>
>> 124:
>> 125: private static Map packageToSystemModule() {
>> 126: List descriptors = ModuleFinder.ofSystem()
>
> Is it a problem that we compute the package -> module map using the runtime
> info (so latest version) but then all the other info is taken from a
> release-specific symbol file? E.g. say that package "foo" was moved from
> module "A" to module "B" in version N, and that user passes N - 1 as release
> to the scan tool - would that work?
That's a good point, I don't think that scenario will work. We should really
use the release specific info if we can. I think that's relatively easy to do,
will take a look.
> src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/JNativeScanTask.java
> line 81:
>
>> 79:
>> 80: Map>>
>> allRestrictedMethods;
>> 81: try(ClassResolver classesToScan =
>> ClassResolver.forScannedModules(modulesToScan, version);
>
> Could it make sense here to "chain" the two resolvers into one, and just pass
> the chained one to the restricted finder?
We need to scan all the classes in the `classesToScan` resolver, but only look
for restricted methods in the system resolver. So, they need to stay separate.
> src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/JNativeScanTask.java
> line 165:
>
>> 163: restrictedUses.forEach(use -> {
>> 164: switch (use) {
>> 165: case
>> RestrictedUse.NativeMethodDecl(MethodRef nmd) ->
>
> should user be able to filter output using an option? E.g. say if they are
> only interested in restricted methods but not JNI (seems remote, but adding
> it here for completeness)
That's a good suggestion, but to pull on that string some more, there's also
the possibility of filtering based on class/package/method names. Maybe for a
followup?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646564392
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646563349
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646566299
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods
On Wed, 19 Jun 2024 17:25:07 GMT, Jorn Vernee wrote:
>> src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/RestrictedMethodFinder.java
>> line 43:
>>
>>> 41: import java.util.*;
>>> 42:
>>> 43: class RestrictedMethodFinder {
>>
>> The name of this is a bit confusing as this class looks for both restricted
>> methods and JNI decls
>
> True. How about `NativeAccessFinder`?
Good name
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646561469
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods
On Tue, 18 Jun 2024 16:30:37 GMT, Jorn Vernee wrote:
> This PR adds a new JDK tool, called `jnativescan`, that can be used to find
> code that accesses native functionality. Currently this includes `native`
> method declarations, and methods marked with `@Restricted`.
>
> The tool accepts a list of class path and module path entries through
> `--class-path` and `--module-path`, and a set of root modules through
> `--add-modules`, as well as an optional target release with `--release`.
>
> The default mode is for the tool to report all uses of `@Restricted` methods,
> and `native` method declaration in a tree-like structure:
>
>
> app.jar (ALL-UNNAMED):
> main.Main:
> main.Main::main(String[])void references restricted methods:
> java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment
> main.Main::m()void is a native method declaration
>
>
> The `--print-native-access` option can be used print out all the module names
> of modules doing native access in a comma separated list. For class path
> code, this will print out `ALL-UNNAMED`.
>
> Testing:
> - `langtools_jnativescan` tests.
> - Running the tool over jextract's libclang bindings, which use the FFM API,
> and thus has a lot of references to `@Restricted` methods.
> - tier 1-3
Looks very good! I left a bunch of comments and question.
src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/ClassResolver.java line
126:
> 124:
> 125: private static Map packageToSystemModule() {
> 126: List descriptors = ModuleFinder.ofSystem()
Is it a problem that we compute the package -> module map using the runtime
info (so latest version) but then all the other info is taken from a
release-specific symbol file? E.g. say that package "foo" was moved from module
"A" to module "B" in version N, and that user passes N - 1 as release to the
scan tool - would that work?
src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/JNativeScanTask.java line
165:
> 163: restrictedUses.forEach(use -> {
> 164: switch (use) {
> 165: case
> RestrictedUse.NativeMethodDecl(MethodRef nmd) ->
should user be able to filter output using an option? E.g. say if they are only
interested in restricted methods but not JNI (seems remote, but adding it here
for completeness)
-
PR Review: https://git.openjdk.org/jdk/pull/19774#pullrequestreview-2128837697
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646558424
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646560383
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods
On Wed, 19 Jun 2024 17:30:08 GMT, Maurizio Cimadamore
wrote:
>> This PR adds a new JDK tool, called `jnativescan`, that can be used to find
>> code that accesses native functionality. Currently this includes `native`
>> method declarations, and methods marked with `@Restricted`.
>>
>> The tool accepts a list of class path and module path entries through
>> `--class-path` and `--module-path`, and a set of root modules through
>> `--add-modules`, as well as an optional target release with `--release`.
>>
>> The default mode is for the tool to report all uses of `@Restricted`
>> methods, and `native` method declaration in a tree-like structure:
>>
>>
>> app.jar (ALL-UNNAMED):
>> main.Main:
>> main.Main::main(String[])void references restricted methods:
>> java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment
>> main.Main::m()void is a native method declaration
>>
>>
>> The `--print-native-access` option can be used print out all the module
>> names of modules doing native access in a comma separated list. For class
>> path code, this will print out `ALL-UNNAMED`.
>>
>> Testing:
>> - `langtools_jnativescan` tests.
>> - Running the tool over jextract's libclang bindings, which use the FFM API,
>> and thus has a lot of references to `@Restricted` methods.
>> - tier 1-3
>
> src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/RestrictedMethodFinder.java
> line 120:
>
>> 118: Optional info =
>> systemClassResolver.lookup(methodRef.owner());
>> 119: if (!info.isPresent()) {
>> 120: return false;
>
> Is this just `false` or maybe a warning that a certain owner could not be
> resolved (maybe if running with enough debug options) ?
Yes, thought about that yesterday as well. Good catch
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646552669
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods
On Tue, 18 Jun 2024 16:30:37 GMT, Jorn Vernee wrote: > This PR adds a new JDK tool, called `jnativescan`, that can be used to find > code that accesses native functionality. Currently this includes `native` > method declarations, and methods marked with `@Restricted`. > > The tool accepts a list of class path and module path entries through > `--class-path` and `--module-path`, and a set of root modules through > `--add-modules`, as well as an optional target release with `--release`. > > The default mode is for the tool to report all uses of `@Restricted` methods, > and `native` method declaration in a tree-like structure: > > > app.jar (ALL-UNNAMED): > main.Main: > main.Main::main(String[])void references restricted methods: > java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment > main.Main::m()void is a native method declaration > > > The `--print-native-access` option can be used print out all the module names > of modules doing native access in a comma separated list. For class path > code, this will print out `ALL-UNNAMED`. > > Testing: > - `langtools_jnativescan` tests. > - Running the tool over jextract's libclang bindings, which use the FFM API, > and thus has a lot of references to `@Restricted` methods. > - tier 1-3 src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/JNativeScanTask.java line 81: > 79: > 80: Map>> > allRestrictedMethods; > 81: try(ClassResolver classesToScan = > ClassResolver.forScannedModules(modulesToScan, version); Could it make sense here to "chain" the two resolvers into one, and just pass the chained one to the restricted finder? - PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646553850
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods
On Tue, 18 Jun 2024 16:30:37 GMT, Jorn Vernee wrote: > This PR adds a new JDK tool, called `jnativescan`, that can be used to find > code that accesses native functionality. Currently this includes `native` > method declarations, and methods marked with `@Restricted`. > > The tool accepts a list of class path and module path entries through > `--class-path` and `--module-path`, and a set of root modules through > `--add-modules`, as well as an optional target release with `--release`. > > The default mode is for the tool to report all uses of `@Restricted` methods, > and `native` method declaration in a tree-like structure: > > > app.jar (ALL-UNNAMED): > main.Main: > main.Main::main(String[])void references restricted methods: > java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment > main.Main::m()void is a native method declaration > > > The `--print-native-access` option can be used print out all the module names > of modules doing native access in a comma separated list. For class path > code, this will print out `ALL-UNNAMED`. > > Testing: > - `langtools_jnativescan` tests. > - Running the tool over jextract's libclang bindings, which use the FFM API, > and thus has a lot of references to `@Restricted` methods. > - tier 1-3 src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/JNativeScanTask.java line 76: > 74: URI location = m.reference().location().orElseThrow(); > 75: Path path = Path.of(location); > 76: checkRegularJar(path); if exploded modules aren't supported (as discussed offline), maybe the error message generated here should be more specific? - PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646550176
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods
On Tue, 18 Jun 2024 16:30:37 GMT, Jorn Vernee wrote:
> This PR adds a new JDK tool, called `jnativescan`, that can be used to find
> code that accesses native functionality. Currently this includes `native`
> method declarations, and methods marked with `@Restricted`.
>
> The tool accepts a list of class path and module path entries through
> `--class-path` and `--module-path`, and a set of root modules through
> `--add-modules`, as well as an optional target release with `--release`.
>
> The default mode is for the tool to report all uses of `@Restricted` methods,
> and `native` method declaration in a tree-like structure:
>
>
> app.jar (ALL-UNNAMED):
> main.Main:
> main.Main::main(String[])void references restricted methods:
> java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment
> main.Main::m()void is a native method declaration
>
>
> The `--print-native-access` option can be used print out all the module names
> of modules doing native access in a comma separated list. For class path
> code, this will print out `ALL-UNNAMED`.
>
> Testing:
> - `langtools_jnativescan` tests.
> - Running the tool over jextract's libclang bindings, which use the FFM API,
> and thus has a lot of references to `@Restricted` methods.
> - tier 1-3
src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/JNativeScanTask.java line
98:
> 96: // in this.classPaths, and recursively following all Class-Path
> manifest
> 97: // attributes
> 98: private Stream findAllClassPathJars() throws
> JNativeScanFatalError {
nice!
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646545209
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods
On Tue, 18 Jun 2024 16:30:37 GMT, Jorn Vernee wrote:
> This PR adds a new JDK tool, called `jnativescan`, that can be used to find
> code that accesses native functionality. Currently this includes `native`
> method declarations, and methods marked with `@Restricted`.
>
> The tool accepts a list of class path and module path entries through
> `--class-path` and `--module-path`, and a set of root modules through
> `--add-modules`, as well as an optional target release with `--release`.
>
> The default mode is for the tool to report all uses of `@Restricted` methods,
> and `native` method declaration in a tree-like structure:
>
>
> app.jar (ALL-UNNAMED):
> main.Main:
> main.Main::main(String[])void references restricted methods:
> java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment
> main.Main::m()void is a native method declaration
>
>
> The `--print-native-access` option can be used print out all the module names
> of modules doing native access in a comma separated list. For class path
> code, this will print out `ALL-UNNAMED`.
>
> Testing:
> - `langtools_jnativescan` tests.
> - Running the tool over jextract's libclang bindings, which use the FFM API,
> and thus has a lot of references to `@Restricted` methods.
> - tier 1-3
src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/RestrictedMethodFinder.java
line 105:
> 103: return switch (method) {
> 104: case MethodRefEntry mre ->
> 105: isRestrictedMethod(mre.owner().asSymbol(),
> mre.name().stringValue(), mre.typeSymbol());
Do we really need these two identical calls to `isRestrictedMethod` ? Note that
both `MethodRefEntry` and `InterfaceMethodEntry` are subclasses of
`MemberRefEntry`, so perhaps the other `isRestrictedMethod` can just take
`MemberRefEntry`?
Or maybe we can have different factories in `MethodRef` one that takes
`InterfaceMethodEntry` and another that takes `MemberRefEntry`.
src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/RestrictedMethodFinder.java
line 120:
> 118: Optional info =
> systemClassResolver.lookup(methodRef.owner());
> 119: if (!info.isPresent()) {
> 120: return false;
Is this just `false` or maybe a warning that a certain owner could not be
resolved (maybe if running with enough debug options) ?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646540627
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646541992
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods
On Tue, 18 Jun 2024 16:30:37 GMT, Jorn Vernee wrote:
> This PR adds a new JDK tool, called `jnativescan`, that can be used to find
> code that accesses native functionality. Currently this includes `native`
> method declarations, and methods marked with `@Restricted`.
>
> The tool accepts a list of class path and module path entries through
> `--class-path` and `--module-path`, and a set of root modules through
> `--add-modules`, as well as an optional target release with `--release`.
>
> The default mode is for the tool to report all uses of `@Restricted` methods,
> and `native` method declaration in a tree-like structure:
>
>
> app.jar (ALL-UNNAMED):
> main.Main:
> main.Main::main(String[])void references restricted methods:
> java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment
> main.Main::m()void is a native method declaration
>
>
> The `--print-native-access` option can be used print out all the module names
> of modules doing native access in a comma separated list. For class path
> code, this will print out `ALL-UNNAMED`.
>
> Testing:
> - `langtools_jnativescan` tests.
> - Running the tool over jextract's libclang bindings, which use the FFM API,
> and thus has a lot of references to `@Restricted` methods.
> - tier 1-3
src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/RestrictedMethodFinder.java
line 43:
> 41: import java.util.*;
> 42:
> 43: class RestrictedMethodFinder {
The name of this is a bit confusing as this class looks for both restricted
methods and JNI decls
src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/RestrictedMethodFinder.java
line 48:
> 46: // see
> make/langtools/src/classes/build/tools/symbolgenerator/CreateSymbols.java
> 47: private static final String RESTRICTED_NAME =
> "Ljdk/internal/javac/Restricted+Annotation;";
> 48: private static final List RESTRICTED_MODULES =
> List.of("java.base");
unused?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646536647
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646535286
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods
On Wed, 19 Jun 2024 17:16:54 GMT, Maurizio Cimadamore
wrote:
>> This PR adds a new JDK tool, called `jnativescan`, that can be used to find
>> code that accesses native functionality. Currently this includes `native`
>> method declarations, and methods marked with `@Restricted`.
>>
>> The tool accepts a list of class path and module path entries through
>> `--class-path` and `--module-path`, and a set of root modules through
>> `--add-modules`, as well as an optional target release with `--release`.
>>
>> The default mode is for the tool to report all uses of `@Restricted`
>> methods, and `native` method declaration in a tree-like structure:
>>
>>
>> app.jar (ALL-UNNAMED):
>> main.Main:
>> main.Main::main(String[])void references restricted methods:
>> java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment
>> main.Main::m()void is a native method declaration
>>
>>
>> The `--print-native-access` option can be used print out all the module
>> names of modules doing native access in a comma separated list. For class
>> path code, this will print out `ALL-UNNAMED`.
>>
>> Testing:
>> - `langtools_jnativescan` tests.
>> - Running the tool over jextract's libclang bindings, which use the FFM API,
>> and thus has a lot of references to `@Restricted` methods.
>> - tier 1-3
>
> src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/Main.java line 126:
>
>> 124: out.println("""
>> 125: The jnativescan tool can be used to find methods that
>> may access native functionality when
>> 126: run. This includes methods that call restricted
>> methods, and 'native' method declarations.
>
> I think it would be more readable if we said "This includes restricted method
> calls and `native` method declarations`. The way it is now it seems (for some
> weird subjective reason) that the sentence is saying "methods ... that call
> ... native method declarations" :-)
Oh, the oxford comma has failed me :D
> src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/RestrictedMethodFinder.java
> line 43:
>
>> 41: import java.util.*;
>> 42:
>> 43: class RestrictedMethodFinder {
>
> The name of this is a bit confusing as this class looks for both restricted
> methods and JNI decls
True. How about `NativeAccessFinder`?
> src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/RestrictedMethodFinder.java
> line 48:
>
>> 46: // see
>> make/langtools/src/classes/build/tools/symbolgenerator/CreateSymbols.java
>> 47: private static final String RESTRICTED_NAME =
>> "Ljdk/internal/javac/Restricted+Annotation;";
>> 48: private static final List RESTRICTED_MODULES =
>> List.of("java.base");
>
> unused?
Yes
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646535247
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646538303
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646535411
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods
On Tue, 18 Jun 2024 16:30:37 GMT, Jorn Vernee wrote:
> This PR adds a new JDK tool, called `jnativescan`, that can be used to find
> code that accesses native functionality. Currently this includes `native`
> method declarations, and methods marked with `@Restricted`.
>
> The tool accepts a list of class path and module path entries through
> `--class-path` and `--module-path`, and a set of root modules through
> `--add-modules`, as well as an optional target release with `--release`.
>
> The default mode is for the tool to report all uses of `@Restricted` methods,
> and `native` method declaration in a tree-like structure:
>
>
> app.jar (ALL-UNNAMED):
> main.Main:
> main.Main::main(String[])void references restricted methods:
> java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment
> main.Main::m()void is a native method declaration
>
>
> The `--print-native-access` option can be used print out all the module names
> of modules doing native access in a comma separated list. For class path
> code, this will print out `ALL-UNNAMED`.
>
> Testing:
> - `langtools_jnativescan` tests.
> - Running the tool over jextract's libclang bindings, which use the FFM API,
> and thus has a lot of references to `@Restricted` methods.
> - tier 1-3
src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/Main.java line 125:
> 123: if (optionSet.has(helpOpt)) {
> 124: out.println("""
> 125: The jnativescan tool can be used to find methods that
> may access native functionality when
Suggestion:
The jnativescan tool can be used to find methods that may
access native functionalities when
src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/Main.java line 126:
> 124: out.println("""
> 125: The jnativescan tool can be used to find methods that
> may access native functionality when
> 126: run. This includes methods that call restricted methods,
> and 'native' method declarations.
I think it would be more readable if we said "This includes restricted method
calls and `native` method declarations`. The way it is now it seems (for some
weird subjective reason) that the sentence is saying "methods ... that call ...
native method declarations" :-)
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646530805
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646531845
Integrated: 8333344: JMX attaching of Subject does not work when security manager not allowed
On Mon, 10 Jun 2024 11:28:28 GMT, Kevin Walls wrote: > JMX uses APIs related to the Security Mananger which are deprecated. Use of > AccessControlContext will be removed when Security Manager is removed. > > Until then, updates are needed to not require setting > -Djava.security.manager=allow to use JMX authentication. This pull request has now been integrated. Changeset: bcf4bb48 Author:Kevin Walls URL: https://git.openjdk.org/jdk/commit/bcf4bb4882e06d8c52f6eb4e9c4e027ba0622c5f Stats: 162 lines in 17 files changed: 113 ins; 3 del; 46 mod 844: JMX attaching of Subject does not work when security manager not allowed Reviewed-by: weijun, dfuchs - PR: https://git.openjdk.org/jdk/pull/19624
Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v19]
On Tue, 18 Jun 2024 12:17:46 GMT, Kevin Walls wrote: >> JMX uses APIs related to the Security Mananger which are deprecated. Use of >> AccessControlContext will be removed when Security Manager is removed. >> >> Until then, updates are needed to not require setting >> -Djava.security.manager=allow to use JMX authentication. > > Kevin Walls has updated the pull request incrementally with one additional > commit since the last revision: > > Additional test runs with SM enabled Integrating, thanks for all the comments. - PR Comment: https://git.openjdk.org/jdk/pull/19624#issuecomment-2179099175
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods
On Wed, 19 Jun 2024 14:30:23 GMT, Roger Riggs wrote: > One more tool. or... Could this be coalesced into a tool that does deprscan > and restricted methods, and other "lint" type checks? I might go so far as to > suggest it be extensible and accept patterns or regular expressions for > matching classes, methods, fields, etc. Discussed this with several people ahead of this PR. That is something we might want to do in the future, but we want this scanning feature to be ready for the JNI restriction, which is proposed to target Java 24. Other work might include unifying the code bases of `jdeprscan`, `jdeps`, and exposing a general purpose static-analysis API. - PR Comment: https://git.openjdk.org/jdk/pull/19774#issuecomment-2179078565
RFR: 8334562: Automate com/sun/security/auth/callback/TextCallbackHandler/Default.java test
The following test: **com/sun/security/auth/callback/TextCallbackHandler/Default.java** is currently marked to be run manually because user inputs are required in the console, but instead it can be automated by providing a custom inputStream to System.in in the actual test to simulate sequential user input. In addition, this patch is removing the test from the problemList as it passes, and from manual test list. - Commit messages: - test TextCallbackHandler/Default.java is automated Changes: https://git.openjdk.org/jdk/pull/19790/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19790=00 Issue: https://bugs.openjdk.org/browse/JDK-8334562 Stats: 50 lines in 3 files changed: 29 ins; 2 del; 19 mod Patch: https://git.openjdk.org/jdk/pull/19790.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19790/head:pull/19790 PR: https://git.openjdk.org/jdk/pull/19790
Re: RFR: 8333268: Fixes for static build [v4]
On Wed, 19 Jun 2024 15:15:43 GMT, Magnus Ihse Bursie wrote: >> This patch contains a set of changes to improve static builds. They will >> pave the way for implementing a full static-only java launcher. The changes >> here will: >> >> 1) Make sure non-exported symbols are made local in the static libraries. >> This means that the risk of symbol conflict is the same for static libraries >> as for dynamic libraries (i.e. in practice zero, as long as a consistent >> naming scheme is used for exported functions). >> >> 2) Remove the work-arounds to exclude duplicated symbols. >> >> 3) Fix some code in hotspot and the JDK libraries that did not work properly >> with a static java launcher. >> >> The latter fixes are copied from or inspired by the work done by >> @jianglizhou and her team as part of the Project Leyden [Hermetic >> Java](https://github.com/openjdk/leyden/tree/hermetic-java-runtime). > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Add dummy implementation of os::lookup_function for Windows The reason the gtest failed was that we build a static library libgtest.a, which is linked with the gtest libjvm.so. With the changes in this PR, libgtest.a was being built using the `ld -r` + `objcopy --localize-hidden` method. This caused some weird issues with gcc, related to C++ code and the `COMDAT` object info. I failed to track down any proper solution, so instead I added a patch where the libraries that we explicitly declare as `STATIC_LIBRARIES` are linked as before, without the partial linking step. These libraries are only intended for internal consumption (that is, they are linked to and used by another, "external" library), and so the extra protection added by the partial linking is not really needed. It's a bit sad that this did not work, but it is no big deal. It won't affect files released in the image, and it will not be a regression as compared to now. - PR Comment: https://git.openjdk.org/jdk/pull/19478#issuecomment-2178961562
Integrated: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles
On Thu, 14 Dec 2023 12:39:52 GMT, Adam Sotona wrote: > java.base java.lang.invoke package heavily uses ASM to generate lambdas and > method handles. > > This patch converts ASM calls to Classfile API. > > This PR is continuation of https://github.com/openjdk/jdk/pull/12945 > > Any comments and suggestions are welcome. > > Please review. > > Thank you, > Adam This pull request has now been integrated. Changeset: 01ee4241 Author:Adam Sotona URL: https://git.openjdk.org/jdk/commit/01ee4241b76e78ca67803c4b083fcedecef1c96c Stats: 2175 lines in 11 files changed: 459 ins; 877 del; 839 mod 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles Co-authored-by: Claes Redestad Reviewed-by: redestad, liach - PR: https://git.openjdk.org/jdk/pull/17108
Re: RFR: 8333268: Fixes for static build [v4]
> This patch contains a set of changes to improve static builds. They will pave > the way for implementing a full static-only java launcher. The changes here > will: > > 1) Make sure non-exported symbols are made local in the static libraries. > This means that the risk of symbol conflict is the same for static libraries > as for dynamic libraries (i.e. in practice zero, as long as a consistent > naming scheme is used for exported functions). > > 2) Remove the work-arounds to exclude duplicated symbols. > > 3) Fix some code in hotspot and the JDK libraries that did not work properly > with a static java launcher. > > The latter fixes are copied from or inspired by the work done by @jianglizhou > and her team as part of the Project Leyden [Hermetic > Java](https://github.com/openjdk/leyden/tree/hermetic-java-runtime). Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Add dummy implementation of os::lookup_function for Windows - Changes: - all: https://git.openjdk.org/jdk/pull/19478/files - new: https://git.openjdk.org/jdk/pull/19478/files/4ab70df3..b88d813e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19478=03 - incr: https://webrevs.openjdk.org/?repo=jdk=19478=02-03 Stats: 6 lines in 1 file changed: 6 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/19478.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19478/head:pull/19478 PR: https://git.openjdk.org/jdk/pull/19478
Re: RFR: 8333268: Fixes for static build [v3]
> This patch contains a set of changes to improve static builds. They will pave > the way for implementing a full static-only java launcher. The changes here > will: > > 1) Make sure non-exported symbols are made local in the static libraries. > This means that the risk of symbol conflict is the same for static libraries > as for dynamic libraries (i.e. in practice zero, as long as a consistent > naming scheme is used for exported functions). > > 2) Remove the work-arounds to exclude duplicated symbols. > > 3) Fix some code in hotspot and the JDK libraries that did not work properly > with a static java launcher. > > The latter fixes are copied from or inspired by the work done by @jianglizhou > and her team as part of the Project Leyden [Hermetic > Java](https://github.com/openjdk/leyden/tree/hermetic-java-runtime). Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Do not use partial linking when building static libraries for internal consumption - Changes: - all: https://git.openjdk.org/jdk/pull/19478/files - new: https://git.openjdk.org/jdk/pull/19478/files/e1c46562..4ab70df3 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19478=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19478=01-02 Stats: 7 lines in 1 file changed: 7 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/19478.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19478/head:pull/19478 PR: https://git.openjdk.org/jdk/pull/19478
Re: RFR: 8334490: Normalize string with locale invariant `toLowerCase()`
On Tue, 18 Jun 2024 18:23:12 GMT, Naoto Sato wrote: > The test library `jdk.test.lib.Platform` uses no-arg `toLowerCase()` for > string comparison, which should be avoided. Marked as reviewed by rriggs (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19775#pullrequestreview-2128480919
Re: RFR: 8309821: Link to hidden classes section in Class specification for Class::isHidden
On Wed, 19 Jun 2024 02:21:05 GMT, Joe Darcy wrote: > Simple doc improvement. Marked as reviewed by rriggs (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19781#pullrequestreview-2128478464
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods
On Tue, 18 Jun 2024 16:30:37 GMT, Jorn Vernee wrote: > This PR adds a new JDK tool, called `jnativescan`, that can be used to find > code that accesses native functionality. Currently this includes `native` > method declarations, and methods marked with `@Restricted`. > > The tool accepts a list of class path and module path entries through > `--class-path` and `--module-path`, and a set of root modules through > `--add-modules`, as well as an optional target release with `--release`. > > The default mode is for the tool to report all uses of `@Restricted` methods, > and `native` method declaration in a tree-like structure: > > > app.jar (ALL-UNNAMED): > main.Main: > main.Main::main(String[])void references restricted methods: > java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment > main.Main::m()void is a native method declaration > > > The `--print-native-access` option can be used print out all the module names > of modules doing native access in a comma separated list. For class path > code, this will print out `ALL-UNNAMED`. > > Testing: > - `langtools_jnativescan` tests. > - Running the tool over jextract's libclang bindings, which use the FFM API, > and thus has a lot of references to `@Restricted` methods. > - tier 1-3 Build changes are trivially fine. - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19774#pullrequestreview-2128470410
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods
On Tue, 18 Jun 2024 16:30:37 GMT, Jorn Vernee wrote: > This PR adds a new JDK tool, called `jnativescan`, that can be used to find > code that accesses native functionality. Currently this includes `native` > method declarations, and methods marked with `@Restricted`. > > The tool accepts a list of class path and module path entries through > `--class-path` and `--module-path`, and a set of root modules through > `--add-modules`, as well as an optional target release with `--release`. > > The default mode is for the tool to report all uses of `@Restricted` methods, > and `native` method declaration in a tree-like structure: > > > app.jar (ALL-UNNAMED): > main.Main: > main.Main::main(String[])void references restricted methods: > java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment > main.Main::m()void is a native method declaration > > > The `--print-native-access` option can be used print out all the module names > of modules doing native access in a comma separated list. For class path > code, this will print out `ALL-UNNAMED`. > > Testing: > - `langtools_jnativescan` tests. > - Running the tool over jextract's libclang bindings, which use the FFM API, > and thus has a lot of references to `@Restricted` methods. > - tier 1-3 One more tool. or... Could this be coalesced into a tool that does deprscan and restricted methods, and other "lint" type checks? I might go so far as to suggest it be extensible and accept patterns or regular expressions for matching classes, methods, fields, etc. - PR Comment: https://git.openjdk.org/jdk/pull/19774#issuecomment-2178860810
Re: RFR: 8333768: Minor doc updates to java.lang.{Float, Double} [v4]
On Wed, 19 Jun 2024 01:49:47 GMT, Joe Darcy wrote: >> src/java.base/share/classes/java/lang/Double.java line 595: >> >>> 593: * This method corresponds to the convertToDecimalCharacter >>> 594: * operation defined in IEEE 754. >>> 595: * >> >> Does it? >> >> IEEE 754 `convertToDecimalCharacter` takes a 2nd argument, the >> `conversionSpecification` which "specifies the precision and formatting of >> the _decimalCharacterSequence_ result". There's no such flexibility here. >> >> Moreover, there seems to be no way to have a `conversionSpecification` that >> ensures the "shortest decimal" semantics of this method. >> >> Finally, IEEE 754 requires to signal inexact exceptions. >> >> Suggestion: >> >> * This method vaguely corresponds to the convertToDecimalCharacter >> >> >> The same holds for the other additional `@apiNote`s. > >> Does it? >> >> IEEE 754 `convertToDecimalCharacter` takes a 2nd argument, the >> `conversionSpecification` which "specifies the precision and formatting of >> the _decimalCharacterSequence_ result". There's no such flexibility here. >> >> Moreover, there seems to be no way to have a `conversionSpecification` that >> ensures the "shortest decimal" semantics of this method. >> >> Finally, IEEE 754 requires to signal inexact exceptions. >> >> The same holds for the other additional `@apiNote`s. > > Thanks for the comments @rgiulietti . > > I've weakened the language for this method and added a javadoc to Formatter, > which more closely aligns with the IEEE model of binary -> decimal > conversion. For toHexString, the IEEE standard does state: > > "When converting to hexadecimal-significand character sequences in the > absence of an explicit precision > specification, enough hexadecimal characters shall be used to represent the > binary floating-point number > exactly." > > so the there isn't an exactly corresponding concern there. > > With the frame condition that the Java platform ignores the sticky flags, I > think making the linkage to IEEE operations is still informative. Basically, > (in many case) if you project down to the subset of IEEE 754 the Java > platform supports, this Java method computes the same floating-point value as > this IEEE operation, etc. Take the `double` closest to the exact decimal 0.1. My understanding is that IEEE with a precision of 17 would convert it to the decimal 0.10001. However, `Formatter` with a specifier `"%.17f"` will render this as 0.1. That's because `Formatter`'s spec is based on this method (which produces 0.1), so cannot generate the trailing 1. Similarly, the `double` closest to 1.2 is converted to 1.19996 by IEEE and to 1.2 by `Formatter`, because this method produces 1.2. In other words, neither this method nor the functionality offered by `Formatter` and friends correspond to IEEE, at least not in full. - PR Review Comment: https://git.openjdk.org/jdk/pull/19590#discussion_r1646298859
Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v25]
On Wed, 19 Jun 2024 09:08:35 GMT, Adam Sotona wrote: >> java.base java.lang.invoke package heavily uses ASM to generate lambdas and >> method handles. >> >> This patch converts ASM calls to Classfile API. >> >> This PR is continuation of https://github.com/openjdk/jdk/pull/12945 >> >> Any comments and suggestions are welcome. >> >> Please review. >> >> Thank you, >> Adam > > Adam Sotona has updated the pull request incrementally with two additional > commits since the last revision: > > - removed empty line > - problem-listed runtime/ClassInitErrors/TestStackOverflowDuringInit.java The new changes since the last round of review is great. Also much thanks for fixing that sneaky condy typo in LMF. - Marked as reviewed by liach (Committer). PR Review: https://git.openjdk.org/jdk/pull/17108#pullrequestreview-2128433285
RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods
This PR adds a new JDK tool, called `jnativescan`, that can be used to find code that accesses native functionality. Currently this includes `native` method declarations, and methods marked with `@Restricted`. The tool accepts a list of class path and module path entries through `--class-path` and `--module-path`, and a set of root modules through `--add-modules`, as well as an optional target release with `--release`. The default mode is for the tool to report all uses of `@Restricted` methods, and `native` method declaration in a tree-like structure: app.jar (ALL-UNNAMED): main.Main: main.Main::main(String[])void references restricted methods: java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment main.Main::m()void is a native method declaration The `--print-native-access` option can be used print out all the module names of modules doing native access in a comma separated list. For class path code, this will print out `ALL-UNNAMED`. Testing: - `langtools_jnativescan` tests. - Running the tool over jextract's libclang bindings, which use the FFM API, and thus has a lot of references to `@Restricted` methods. - tier 1-3 - Commit messages: - Merge branch 'master' into jnativescan - use URI-based constructor of Path - add jnativescan to help flag test - add --print-native-access test - correct one more comment - correct comment - add missing newlinw - add more testing + cleanup code - add test for references through subclasses - add test for array method refs - ... and 5 more: https://git.openjdk.org/jdk/compare/50bed6c6...9e8dad03 Changes: https://git.openjdk.org/jdk/pull/19774/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19774=00 Issue: https://bugs.openjdk.org/browse/JDK-8317611 Stats: 1769 lines in 34 files changed: 1760 ins; 0 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/19774.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19774/head:pull/19774 PR: https://git.openjdk.org/jdk/pull/19774
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods
On Tue, 18 Jun 2024 16:30:37 GMT, Jorn Vernee wrote:
> This PR adds a new JDK tool, called `jnativescan`, that can be used to find
> code that accesses native functionality. Currently this includes `native`
> method declarations, and methods marked with `@Restricted`.
>
> The tool accepts a list of class path and module path entries through
> `--class-path` and `--module-path`, and a set of root modules through
> `--add-modules`, as well as an optional target release with `--release`.
>
> The default mode is for the tool to report all uses of `@Restricted` methods,
> and `native` method declaration in a tree-like structure:
>
>
> app.jar (ALL-UNNAMED):
> main.Main:
> main.Main::main(String[])void references restricted methods:
> java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment
> main.Main::m()void is a native method declaration
>
>
> The `--print-native-access` option can be used print out all the module names
> of modules doing native access in a comma separated list. For class path
> code, this will print out `ALL-UNNAMED`.
>
> Testing:
> - `langtools_jnativescan` tests.
> - Running the tool over jextract's libclang bindings, which use the FFM API,
> and thus has a lot of references to `@Restricted` methods.
> - tier 1-3
src/jdk.compiler/share/classes/com/sun/tools/javac/platform/JDKPlatformProvider.java
line 93:
> 91: }
> 92:
> 93: public PlatformDescription getPlatformTrusted(String platformName) {
I noticed that `getPlatform` was not throwing an exception if the release was
not valid, which then later results in an NPE. I've added an explicit check
here instead. The caller can then catch the exception.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1644763290
Re: RFR: 8332842: Optimize empty CopyOnWriteArrayList allocations [v3]
On Wed, 19 Jun 2024 11:58:07 GMT, Aleksey Shipilev wrote: > @AlanBateman -- could you please take a look? Thanks. There was a lot of heap analysis done a few years ago that shined a light on the number of empty collections in a typical heap. I don't recall seeing COWAL in any of the data but it seems plausible that there are cases where there are a lot of empty COWAL instances in the heap. Main thing for a change like this to make sure it doesn't hurt other cases and check if there are overridable methods used in the existing + updated implementations that might get reported as a behavior change by something that extends and overrides some methods. I don't see anything so I think it's okay. One thing that surprises me is the change to remove(Object) to handle the case where the last remaining element is removed. Does that help the application that prompted this changed? Part of wonders if remove(int) needs to do the same as someone will show up sometime to ask. - PR Comment: https://git.openjdk.org/jdk/pull/19527#issuecomment-2178649021
Re: RFR: 8307818: Convert Indify tool to Classfile API [v9]
On Mon, 17 Jun 2024 08:12:08 GMT, Adam Sotona wrote:
>> Oussama Louati has updated the pull request incrementally with 19 additional
>> commits since the last revision:
>>
>> - fix: fixed whitespaces
>> - fix: fixed whitespaces
>> - chore: used Class::descriptorString
>> - remove: added removed test comments
>> - remove: added removed test comments
>> - remove: removed changes in tests
>> - update: optimize imports and remove unnecessary utils
>> - update: optimize imports
>> - update: 5th patch of code review
>> - update: 5th patch of code review
>> - ... and 9 more: https://git.openjdk.org/jdk/compare/781c951d...2b8c94a7
>
> test/jdk/java/lang/invoke/indify/Indify.java line 197:
>
>> 195: try {
>> 196: runApplication(avl.toArray(new String[0]));
>> 197: } catch (Exception ex) {
>
> Why the comments have been removed?
Detailed explanations are above in those options variables declaration.
Comments returned
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1646069101
Re: RFR: 8307818: Convert Indify tool to Classfile API [v11]
> An indify tool in j.l.i tests (also in vmTestBase) convert some source-code > private static methods with MT_ MH_, and INDY_ prefixes into MethodHandle, > MethodType, and CallSite constants. > It currently uses ad-hoc code to process class files and intends to migrate > to ASM; but since we have the Classfile API, we can migrate to Classfile API > instead. Oussama Louati has updated the pull request incrementally with one additional commit since the last revision: fix: fix comments - Changes: - all: https://git.openjdk.org/jdk/pull/18841/files - new: https://git.openjdk.org/jdk/pull/18841/files/0cac912d..f869cce8 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18841=10 - incr: https://webrevs.openjdk.org/?repo=jdk=18841=09-10 Stats: 6 lines in 1 file changed: 0 ins; 0 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/18841.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18841/head:pull/18841 PR: https://git.openjdk.org/jdk/pull/18841
Re: RFR: 8332842: Optimize empty CopyOnWriteArrayList allocations [v3]
On Thu, 6 Jun 2024 12:46:36 GMT, jengebr wrote: >> Improve `java/util/concurrent/CopyOnWriteArrayList` by eliminating needless >> cloning of Object[0] instances. This cloning is intended to prevent callers >> from changing array contents, but many `CopyOnWriteArrayList`s are allocated >> to size zero, or are otherwise maintained empty, so cloning is unnecessary. >> >> Results from the included JMH benchmark: >> Before: >> >> BenchmarkMode Cnt >> Score Error Units >> CopyOnWriteArrayListBenchmark.clear avgt5 >> 74.487 ± 1.793 ns/op >> CopyOnWriteArrayListBenchmark.clearEmpty avgt5 >> 27.918 ± 0.759 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceArrayavgt5 >> 16.656 ± 0.375 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceArrayEmpty avgt5 >> 15.415 ± 0.489 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceCollection avgt5 >> 21.608 ± 0.363 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceCollectionEmpty avgt5 >> 15.374 ± 0.260 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceDefault avgt5 >> 15.688 ± 0.350 ns/op >> CopyOnWriteArrayListBenchmark.readInstance avgt 10 >> 2625.125 ± 71.802 ns/op >> CopyOnWriteArrayListBenchmark.readInstanceEmpty avgt 10 >> 2607.447 ± 46.400 ns/op >> >> >> After: >> >> BenchmarkMode Cnt >> Score Error Units >> CopyOnWriteArrayListBenchmark.clear avgt5 >> 75.365 ± 2.092 ns/op >> CopyOnWriteArrayListBenchmark.clearEmpty avgt5 >> 20.803 ± 0.539 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceArrayavgt5 >> 16.808 ± 0.582 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceArrayEmpty avgt5 >> 12.980 ± 0.418 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceCollection avgt5 >> 21.627 ± 0.173 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceCollectionEmpty avgt5 >> 12.864 ± 0.408 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceDefault avgt5 >> 12.931 ± 0.255 ns/op >> CopyOnWriteArrayListBenchmark.readInstance avgt 10 >> 2615.500 ± 30.771 ns/op >> CopyOnWriteArrayListBenchmark.readInstanceEmpty avgt 10 >> 2583.892 ± 62.086 ns/op > > jengebr has updated the pull request incrementally with one additional commit > since the last revision: > > Adding benchmarks for readObject @AlanBateman -- could you please take a look? Thanks. - PR Comment: https://git.openjdk.org/jdk/pull/19527#issuecomment-2178515650
Withdrawn: 8315487: Security Providers Filter
On Fri, 1 Sep 2023 15:13:46 GMT, Martin Balao wrote: > In addition to the goals, scope, motivation, specification and requirement > notes in [JDK-8315487](https://bugs.openjdk.org/browse/JDK-8315487), we would > like to describe the most relevant decisions taken during the implementation > of this enhancement. These notes are organized by feature, may encompass more > than one file or code segment, and are aimed to provide a high-level view of > this PR. > > ## ProvidersFilter > > ### Filter construction (parser) > > The providers filter is constructed from a string value, taken from either a > system or a security property with name "jdk.security.providers.filter". This > process occurs at sun.security.jca.ProvidersFilter class —simply referred as > ProvidersFilter onward— static initialization. Thus, changes to the filter's > overridable property are not effective afterwards and no assumptions should > be made regarding when this class gets initialized. > > The filter's string value is processed with a custom parser of order 'n', > being 'n' the number of characters. The parser, represented by the > ProvidersFilter.Parser class, can be characterized as a Deterministic Finite > Automaton (DFA). The ProvidersFilter.Parser::parse method is the starting > point to get characters from the filter's string value and generate state > transitions in the parser's internal state-machine. See > ProvidersFilter.Parser::nextState for more details about the parser's states > and both valid and invalid transitions. The ParsingState enum defines valid > parser states and Transition the reasons to move between states. If a filter > string cannot be parsed, a ProvidersFilter.ParserException exception is > thrown, and turned into an unchecked IllegalArgumentException in the > ProvidersFilter.Filter constructor. > > While we analyzed —and even tried, at early stages of the development— the > use of regular expressions for filter parsing, we discarded the approach in > order to get maximum performance, support a more advanced syntax and have > flexibility for further extensions in the future. > > ### Filter (structure and behavior) > > A filter is represented by the ProvidersFilter.Filter class. It consists of > an ordered list of rules, returned by the parser, that represents filter > patterns from left to right (see the filter syntax for reference). At the end > of this list, a match-all and deny rule is added for default behavior. When a > service is evaluated against the filter, each filter rule is checked in the > ProvidersFilter.Filter::apply method. The rule makes an allow or deny > decision if the ser... This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/15539
Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v19]
On Wed, 19 Jun 2024 11:21:45 GMT, Daniel Fuchs wrote: > The code changes look good to me (if a bit verbose) and the test changes look > reasonable. It could be beneficial to add some more tests in the future > involving monitoring and getting the subject from within a monitored MBean. Yes, agreed. Thanks. - PR Comment: https://git.openjdk.org/jdk/pull/19624#issuecomment-2178461246
Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v19]
On Tue, 18 Jun 2024 12:17:46 GMT, Kevin Walls wrote: >> JMX uses APIs related to the Security Mananger which are deprecated. Use of >> AccessControlContext will be removed when Security Manager is removed. >> >> Until then, updates are needed to not require setting >> -Djava.security.manager=allow to use JMX authentication. > > Kevin Walls has updated the pull request incrementally with one additional > commit since the last revision: > > Additional test runs with SM enabled The code changes look good to me (if a bit verbose) and the test changes look reasonable. It could be beneficial to add some more tests in the future involving monitoring and getting the subject from within a monitored MBean. - Marked as reviewed by dfuchs (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19624#pullrequestreview-2127943873
Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v25]
On Wed, 19 Jun 2024 09:08:35 GMT, Adam Sotona wrote: >> java.base java.lang.invoke package heavily uses ASM to generate lambdas and >> method handles. >> >> This patch converts ASM calls to Classfile API. >> >> This PR is continuation of https://github.com/openjdk/jdk/pull/12945 >> >> Any comments and suggestions are welcome. >> >> Please review. >> >> Thank you, >> Adam > > Adam Sotona has updated the pull request incrementally with two additional > commits since the last revision: > > - removed empty line > - problem-listed runtime/ClassInitErrors/TestStackOverflowDuringInit.java No objections as long as there's an understanding that we'll need to work on some of the startup/warmup regressions this will cause. We have preliminary data on a subset of benchmarks which suggests it's a combination of loading more classes and spreading the work across a larger set of methods, which means we need to warm and JIT more methods to reach peak performance. ASM is a smaller library with a more minimalist approach, so it loads faster and gets up and running a bit faster. We'll probably need to file regressions as they are detected for tracking purposes, but I expect we'll continue chipping away at and improving the classfile API in the months to come. - Marked as reviewed by redestad (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17108#pullrequestreview-2127764187
Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v25]
On Wed, 19 Jun 2024 09:08:35 GMT, Adam Sotona wrote: >> java.base java.lang.invoke package heavily uses ASM to generate lambdas and >> method handles. >> >> This patch converts ASM calls to Classfile API. >> >> This PR is continuation of https://github.com/openjdk/jdk/pull/12945 >> >> Any comments and suggestions are welcome. >> >> Please review. >> >> Thank you, >> Adam > > Adam Sotona has updated the pull request incrementally with two additional > commits since the last revision: > > - removed empty line > - problem-listed runtime/ClassInitErrors/TestStackOverflowDuringInit.java `runtime/ClassInitErrors/TestStackOverflowDuringInit.java` seems to be fragile and affected by this PR, so I created [JDK-8334545](https://bugs.openjdk.org/browse/JDK-8334545) and listed the test in `test/hotspot/jtreg/ProblemList.txt` @mlchung @cl4es @liach @ExE-Boss many thanks for significant contribution to this complex conversion! Any final thoughts or objections to the integration? Thank you! - PR Comment: https://git.openjdk.org/jdk/pull/17108#issuecomment-2178180439
Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v25]
> java.base java.lang.invoke package heavily uses ASM to generate lambdas and > method handles. > > This patch converts ASM calls to Classfile API. > > This PR is continuation of https://github.com/openjdk/jdk/pull/12945 > > Any comments and suggestions are welcome. > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with two additional commits since the last revision: - removed empty line - problem-listed runtime/ClassInitErrors/TestStackOverflowDuringInit.java - Changes: - all: https://git.openjdk.org/jdk/pull/17108/files - new: https://git.openjdk.org/jdk/pull/17108/files/257d66ee..6e851a50 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17108=24 - incr: https://webrevs.openjdk.org/?repo=jdk=17108=23-24 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/17108.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17108/head:pull/17108 PR: https://git.openjdk.org/jdk/pull/17108
Withdrawn: 8329335: HttpsURLConnectionTest fails due to network firewall rules
On Mon, 17 Jun 2024 09:16:45 GMT, Fernando Guallini wrote: > Since HttpsURLConnectionTest attempts to reach external servers, it can fail > if run on hosts without outbound traffic allowed. Therefore, it should not be > executed in CI pipelines but rather manually on a host with no firewall rules > preventing egress traffic. This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/19745
Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v24]
> java.base java.lang.invoke package heavily uses ASM to generate lambdas and > method handles. > > This patch converts ASM calls to Classfile API. > > This PR is continuation of https://github.com/openjdk/jdk/pull/12945 > > Any comments and suggestions are welcome. > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: fixed sneaky completion typo - Changes: - all: https://git.openjdk.org/jdk/pull/17108/files - new: https://git.openjdk.org/jdk/pull/17108/files/ac20f1f8..257d66ee Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17108=23 - incr: https://webrevs.openjdk.org/?repo=jdk=17108=22-23 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17108.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17108/head:pull/17108 PR: https://git.openjdk.org/jdk/pull/17108
Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v23]
> java.base java.lang.invoke package heavily uses ASM to generate lambdas and > method handles. > > This patch converts ASM calls to Classfile API. > > This PR is continuation of https://github.com/openjdk/jdk/pull/12945 > > Any comments and suggestions are welcome. > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: Update src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com> - Changes: - all: https://git.openjdk.org/jdk/pull/17108/files - new: https://git.openjdk.org/jdk/pull/17108/files/857b8821..ac20f1f8 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17108=22 - incr: https://webrevs.openjdk.org/?repo=jdk=17108=21-22 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/17108.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17108/head:pull/17108 PR: https://git.openjdk.org/jdk/pull/17108
