Re: RFR: JDK-8027711: Unify wildcarding syntax for CompileCommand and CompileOnly [v2]
On Tue, 23 May 2023 09:08:20 GMT, Tobias Holenstein wrote: >> At the moment `CompileCommand` and `CompileOnly` use different syntax for >> matching methods. >> >> ### Old CompileOnly format >> - matching a **method name** with **class name** and **package name**: >> `-XX:CompileOnly=package/path/Class.method` >> `-XX:CompileOnly=package/path/Class::method` >> `-XX:CompileOnly=package.path.Class::method` >> BUT NOT `-XX:CompileOnly=package.path.Class.method` >> >> - just matching a **single method name**: >> `-XX:CompileOnly=.hashCode` >> `-XX:CompileOnly=::hashCode` >> BUT NOT `-XX:CompileOnly=hashCode` >> >> - Matching **all method names** in a **class name** with **package name** >> `-XX:CompileOnly=java/lang/String` >> BUT NOT `-XX:CompileOnly=java/lang/String.` >> BUT NOT `-XX:CompileOnly=java.lang.String` >> BUT NOT `-XX:CompileOnly=java.lang.String::` (This is actually a bug) >> BUT NOT `-XX:CompileOnly=String` >> BUT NOT `-XX:CompileOnly=String.` >> BUT NOT `-XX:CompileOnly=String::` >> >> - Matching **all method names** in a **class name** with **NO package name** >> `-XX:CompileOnly=String` >> BUT NOT `-XX:CompileOnly=String.` >> BUT NOT `-XX:CompileOnly=String::` >> >> - There is a bug when `CompileOnly` ends with `::` where the `CompileOnly` >> is just ignored >> e.g. `-XX:CompileOnly=String::` compiles as many methods as when omitting >> the `-XX:CompileOnly=` command >> >> ### CompileCommand=compileonly format >> `CompileCommand` allows two different forms for paths: >> - `package/path/Class.method` >> - `package.path.Class::method` >> >> In contrary to `CompileOnly` `CompileCommand` supports wildcard matching >> using `*`. `*` can appear at the beginning and/or end of a >> `package.path.Class` and `method` name. >> >> Valid forms: >> `-XX:CompileCommand=compileonly,*.lang.*::*shCo*` >> `-XX:CompileCommand=compileonly,*/lang/*.*shCo*` >> `-XX:CompileCommand=compileonly,java.lang.String::*` >> `-XX:CompileCommand=compileonly,*::hashCode` >> `-XX:CompileCommand=compileonly,*ng.String::hashC*` >> `-XX:CompileCommand=compileonly,*String::hash*` >> >> Invalid forms (Error: Embedded * not allowed): >> `-XX:CompileCommand=compileonly,java.*.String::has*Code` >> >> ### Use CompileCommand syntax for CompileOnly >> At the moment, in some cases it is not possible to just take pattern used >> with `CompileOnly` and plug it into compile command file. Syntax used by >> CompileOnly is also not very intuitive. >> >> `CompileOnly` is convenient because it's shorter to write and takes lists of >> patterns, whereas `CompileCommand` only takes one pattern per command. >> >> W... > > Tobias Holenstein has updated the pull request incrementally with three > additional commits since the last revision: > > - Update Test8211698.java > - Update src/hotspot/share/compiler/compilerOracle.cpp > >Co-authored-by: Christian Hagedorn > - Update src/hotspot/share/compiler/compilerOracle.cpp > >Co-authored-by: Christian Hagedorn Update looks good! - Marked as reviewed by chagedorn (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13802#pullrequestreview-1452864244
Re: RFR: JDK-8027711: Unify wildcarding syntax for CompileCommand and CompileOnly [v2]
On Tue, 23 May 2023 07:55:35 GMT, Christian Hagedorn
wrote:
>> Tobias Holenstein has updated the pull request incrementally with three
>> additional commits since the last revision:
>>
>> - Update Test8211698.java
>> - Update src/hotspot/share/compiler/compilerOracle.cpp
>>
>>Co-authored-by: Christian Hagedorn
>> - Update src/hotspot/share/compiler/compilerOracle.cpp
>>
>>Co-authored-by: Christian Hagedorn
>
> src/hotspot/share/compiler/compilerOracle.cpp line 1017:
>
>> 1015: if (line[0] == '\0') return;
>> 1016: ResourceMark rm;
>> 1017: char error_buf[1024] = {0};
>
> Wouldn't it be sufficient to only initialize the first character with `\0`?
Probably, but all other error buffer also init with zero. I will leave it for
now. Can update them all together in the future
-
PR Review Comment: https://git.openjdk.org/jdk/pull/13802#discussion_r1203930148
Re: RFR: JDK-8027711: Unify wildcarding syntax for CompileCommand and CompileOnly [v2]
> At the moment `CompileCommand` and `CompileOnly` use different syntax for > matching methods. > > ### Old CompileOnly format > - matching a **method name** with **class name** and **package name**: > `-XX:CompileOnly=package/path/Class.method` > `-XX:CompileOnly=package/path/Class::method` > `-XX:CompileOnly=package.path.Class::method` > BUT NOT `-XX:CompileOnly=package.path.Class.method` > > - just matching a **single method name**: > `-XX:CompileOnly=.hashCode` > `-XX:CompileOnly=::hashCode` > BUT NOT `-XX:CompileOnly=hashCode` > > - Matching **all method names** in a **class name** with **package name** > `-XX:CompileOnly=java/lang/String` > BUT NOT `-XX:CompileOnly=java/lang/String.` > BUT NOT `-XX:CompileOnly=java.lang.String` > BUT NOT `-XX:CompileOnly=java.lang.String::` (This is actually a bug) > BUT NOT `-XX:CompileOnly=String` > BUT NOT `-XX:CompileOnly=String.` > BUT NOT `-XX:CompileOnly=String::` > > - Matching **all method names** in a **class name** with **NO package name** > `-XX:CompileOnly=String` > BUT NOT `-XX:CompileOnly=String.` > BUT NOT `-XX:CompileOnly=String::` > > - There is a bug when `CompileOnly` ends with `::` where the `CompileOnly` is > just ignored > e.g. `-XX:CompileOnly=String::` compiles as many methods as when omitting the > `-XX:CompileOnly=` command > > ### CompileCommand=compileonly format > `CompileCommand` allows two different forms for paths: > - `package/path/Class.method` > - `package.path.Class::method` > > In contrary to `CompileOnly` `CompileCommand` supports wildcard matching > using `*`. `*` can appear at the beginning and/or end of a > `package.path.Class` and `method` name. > > Valid forms: > `-XX:CompileCommand=compileonly,*.lang.*::*shCo*` > `-XX:CompileCommand=compileonly,*/lang/*.*shCo*` > `-XX:CompileCommand=compileonly,java.lang.String::*` > `-XX:CompileCommand=compileonly,*::hashCode` > `-XX:CompileCommand=compileonly,*ng.String::hashC*` > `-XX:CompileCommand=compileonly,*String::hash*` > > Invalid forms (Error: Embedded * not allowed): > `-XX:CompileCommand=compileonly,java.*.String::has*Code` > > ### Use CompileCommand syntax for CompileOnly > At the moment, in some cases it is not possible to just take pattern used > with `CompileOnly` and plug it into compile command file. Syntax used by > CompileOnly is also not very intuitive. > > `CompileOnly` is convenient because it's shorter to write and takes lists of > patterns, whereas `CompileCommand` only takes one pattern per command. > > With this PR `CompileOnly` becomes an alias for `CompileC... Tobias Holenstein has updated the pull request incrementally with three additional commits since the last revision: - Update Test8211698.java - Update src/hotspot/share/compiler/compilerOracle.cpp Co-authored-by: Christian Hagedorn - Update src/hotspot/share/compiler/compilerOracle.cpp Co-authored-by: Christian Hagedorn - Changes: - all: https://git.openjdk.org/jdk/pull/13802/files - new: https://git.openjdk.org/jdk/pull/13802/files/d6fca2b8..40b17296 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=13802=01 - incr: https://webrevs.openjdk.org/?repo=jdk=13802=00-01 Stats: 7 lines in 2 files changed: 5 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/13802.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13802/head:pull/13802 PR: https://git.openjdk.org/jdk/pull/13802
Re: RFR: JDK-8027711: Unify wildcarding syntax for CompileCommand and CompileOnly [v2]
On Tue, 23 May 2023 08:01:07 GMT, Christian Hagedorn wrote: >> Tobias Holenstein has updated the pull request incrementally with three >> additional commits since the last revision: >> >> - Update Test8211698.java >> - Update src/hotspot/share/compiler/compilerOracle.cpp >> >>Co-authored-by: Christian Hagedorn >> - Update src/hotspot/share/compiler/compilerOracle.cpp >> >>Co-authored-by: Christian Hagedorn > > test/hotspot/jtreg/compiler/loopopts/Test8211698.java line 57: > >> 55: } >> 56: } >> 57: > > You can leave this empty line in at the end of the file. done - PR Review Comment: https://git.openjdk.org/jdk/pull/13802#discussion_r1201860361
