Re: RFR: JDK-8236138: Add tests for jmod applications
Code looks fine, I did not run the tests, but +1 assuming this has been run through a mach5 test job already. If not please submit one first. -phil. On 12/18/19, 6:38 AM, Andy Herrick wrote: Code looks good - ran tests locally and all passed. /Andyu On 12/17/2019 8:31 PM, Alexey Semenyuk wrote: Please review fix [2] for jpackage bug [1]. The fix updates jtreg tests by adding test coverage for scenarios when jpackage is used for packaging applications bundled in .jmod files. - Alexey [1] https://bugs.openjdk.java.net/browse/JDK-8236138 [2] http://cr.openjdk.java.net/~asemenyuk/8236138/webrev.00
Re: RFR: [XS]: 8236183: cleanup Java_jdk_internal_reflect_Reflection_getCallerClass naming - was : RE: running java with LD_DEBUG-tracing
Hi Matthias, Looks good. Thanks for fixing and finding. :) David On 19/12/2019 12:33 am, Baesken, Matthias wrote: Hello David, Here is a change that adjusts the naming of Java_jdk_internal_reflect_Reflection_getCallerClass - With this change, I checked the output of LD_DEBUG=libs java Test , and the error I observed before is gone . Bug/webrev : https://bugs.openjdk.java.net/browse/JDK-8236183 http://cr.openjdk.java.net/~mbaesken/webrevs/8236183.0/ Thanks, Matthias By chance I noticed the following output when running a trivial Java program : LD_DEBUG=libs /linuxx86_64/output-jdk- test/images/jdk/bin/java Test . . . 12241: 12241: /linuxx86_64/output-jdk-test/images/jdk/lib/libjava.so: error: symbol lookup error: undefined symbol: Java_jdk_internal_reflect_Reflection_getCallerClass (fatal) 12241: What is wrong with "Java_jdk_internal_reflect_Reflection_getCallerClass" ? Does the error message occur because of the naming Java_jdk_internal_reflect_Reflection_getCallerClass__ with the added "__" at the end of the method (Reflection.c) ? That definitely looks like an error in the C source to me. The ending "__" should not be there. How is this working ?? Ah! It is a remnant of when there were overloads of getCallerClass. The "__" should be followed by the signature - which is empty in this case. So IIRC the lookup will first try to use the short name (without the trailing "__") which will fail, then it will use the long name which will succeed. It should be fixed so we don't waste time doing two lookups. David
Re: RFR 8225466 : Optimize matching BMP Slice nodes
Hi Roger.
Thank you for taking a look!
The variant with a single loop was the first thing I tried (should have
mentioned that in the review request).
Unfortunately, that showed performance decrease.
I suspect that hitting the end of the input should be a less common
thing, that's why it it beneficial to separate it as a slow path.
With kind regards,
Ivan
On 12/18/19 7:48 AM, Roger Riggs wrote:
Hi Ivan,
Though the new code has a good effect, the asymmetry and duplication
seems unnecessary.
Can it be structured to have a single copy of the loop comparing the
available range
and still get the desired performance improvement.
Like:
boolean match(Matcher matcher,int i, CharSequence seq) {
int[] buf =buffer;
int len = buf.length;
for (int j =0; j < Math.min(len, matcher.to); j++) {
if (buf[j] != seq.charAt(i+j))
return false;
}
if (len >= matcher.to) {
matcher.hitEnd =true;
return false;
}
return next.match(matcher, i+len, seq);
}
Regards, Roger
On 10/28/19 9:03 PM, Ivan Gerasimov wrote:
Hello!
When building a Pattern object, the regex parser recognizes "slices"
- continuous char subsequences, which all have to be matched
case-sensitively/case-insensitively. Matching with such a slice is
implemented as a simple loop over a portion of the input.
In the current implementation, on each iteration of the loop it is
checked if we have hit the end of the input (which is an uncommon case).
This check can be done only once, before the loop, which will make
the loop lighter.
Benchmark shows up to +4% to the throughput for the case-insensitive
matching.
Would you please help review the enhancement?
BUGURL: https://bugs.openjdk.java.net/browse/JDK-8225466
WEBREV: http://cr.openjdk.java.net/~igerasim/8225466/00/webrev/
--- benchmark results ---
UNFIXED
Benchmark Mode Cnt Score Error Units
PatternBench.sliceIFind avgt 16 190.612 ? 0.336 ns/op
FIXED
Benchmark Mode Cnt Score Error Units
PatternBench.sliceIFind avgt 16 182.954 ? 0.493 ns/op
---
--
With kind regards,
Ivan Gerasimov
Re: RFR 8225466 : Optimize matching BMP Slice nodes
Hi Ivan,
Though the new code has a good effect, the asymmetry and duplication
seems unnecessary.
Can it be structured to have a single copy of the loop comparing the
available range
and still get the desired performance improvement.
Like:
boolean match(Matcher matcher,int i, CharSequence seq) {
int[] buf =buffer;
int len = buf.length;
for (int j =0; j < Math.min(len, matcher.to); j++) {
if (buf[j] != seq.charAt(i+j))
return false;
}
if (len >= matcher.to) {
matcher.hitEnd =true;
return false;
}
return next.match(matcher, i+len, seq);
}
Regards, Roger
On 10/28/19 9:03 PM, Ivan Gerasimov wrote:
Hello!
When building a Pattern object, the regex parser recognizes "slices" -
continuous char subsequences, which all have to be matched
case-sensitively/case-insensitively. Matching with such a slice is
implemented as a simple loop over a portion of the input.
In the current implementation, on each iteration of the loop it is
checked if we have hit the end of the input (which is an uncommon case).
This check can be done only once, before the loop, which will make the
loop lighter.
Benchmark shows up to +4% to the throughput for the case-insensitive
matching.
Would you please help review the enhancement?
BUGURL: https://bugs.openjdk.java.net/browse/JDK-8225466
WEBREV: http://cr.openjdk.java.net/~igerasim/8225466/00/webrev/
--- benchmark results ---
UNFIXED
Benchmark Mode Cnt Score Error Units
PatternBench.sliceIFind avgt 16 190.612 ? 0.336 ns/op
FIXED
Benchmark Mode Cnt Score Error Units
PatternBench.sliceIFind avgt 16 182.954 ? 0.493 ns/op
---
Re: RFR: [XS]: 8236183: cleanup Java_jdk_internal_reflect_Reflection_getCallerClass naming - was : RE: running java with LD_DEBUG-tracing
On 18/12/2019 14:33, Baesken, Matthias wrote: Hello David, Here is a change that adjusts the naming of Java_jdk_internal_reflect_Reflection_getCallerClass - With this change, I checked the output of LD_DEBUG=libs java Test , and the error I observed before is gone . Bug/webrev : https://bugs.openjdk.java.net/browse/JDK-8236183 http://cr.openjdk.java.net/~mbaesken/webrevs/8236183.0/ Good sleuthing, this change looks good. -Alan.
Re: RFR: JDK-8236138: Add tests for jmod applications
Code looks good - ran tests locally and all passed. /Andyu On 12/17/2019 8:31 PM, Alexey Semenyuk wrote: Please review fix [2] for jpackage bug [1]. The fix updates jtreg tests by adding test coverage for scenarios when jpackage is used for packaging applications bundled in .jmod files. - Alexey [1] https://bugs.openjdk.java.net/browse/JDK-8236138 [2] http://cr.openjdk.java.net/~asemenyuk/8236138/webrev.00
Re: RFR: [XS]: 8236183: cleanup Java_jdk_internal_reflect_Reflection_getCallerClass naming - was : RE: running java with LD_DEBUG-tracing
Nice find! Looks good to me. /Claes On 2019-12-18 15:33, Baesken, Matthias wrote: Hello David, Here is a change that adjusts the naming of Java_jdk_internal_reflect_Reflection_getCallerClass - With this change, I checked the output of LD_DEBUG=libs java Test , and the error I observed before is gone . Bug/webrev : https://bugs.openjdk.java.net/browse/JDK-8236183 http://cr.openjdk.java.net/~mbaesken/webrevs/8236183.0/ Thanks, Matthias By chance I noticed the following output when running a trivial Java program : LD_DEBUG=libs /linuxx86_64/output-jdk- test/images/jdk/bin/java Test . . . 12241: 12241: /linuxx86_64/output-jdk-test/images/jdk/lib/libjava.so: error: symbol lookup error: undefined symbol: Java_jdk_internal_reflect_Reflection_getCallerClass (fatal) 12241: What is wrong with "Java_jdk_internal_reflect_Reflection_getCallerClass" ? Does the error message occur because of the naming Java_jdk_internal_reflect_Reflection_getCallerClass__ with the added "__" at the end of the method (Reflection.c) ? That definitely looks like an error in the C source to me. The ending "__" should not be there. How is this working ?? Ah! It is a remnant of when there were overloads of getCallerClass. The "__" should be followed by the signature - which is empty in this case. So IIRC the lookup will first try to use the short name (without the trailing "__") which will fail, then it will use the long name which will succeed. It should be fixed so we don't waste time doing two lookups. David
RFR: [XS]: 8236183: cleanup Java_jdk_internal_reflect_Reflection_getCallerClass naming - was : RE: running java with LD_DEBUG-tracing
Hello David, Here is a change that adjusts the naming of Java_jdk_internal_reflect_Reflection_getCallerClass - With this change, I checked the output of LD_DEBUG=libs java Test , and the error I observed before is gone . Bug/webrev : https://bugs.openjdk.java.net/browse/JDK-8236183 http://cr.openjdk.java.net/~mbaesken/webrevs/8236183.0/ Thanks, Matthias > >> > >> By chance I noticed the following output when running a trivial Java > >> program : > >> > >> LD_DEBUG=libs /linuxx86_64/output-jdk- > test/images/jdk/bin/java Test > >> . . . > >> 12241: > >> 12241: > >> /linuxx86_64/output-jdk-test/images/jdk/lib/libjava.so: error: symbol > >> lookup error: undefined symbol: > >> Java_jdk_internal_reflect_Reflection_getCallerClass (fatal) > >> 12241: > >> > >> What is wrong with > >> "Java_jdk_internal_reflect_Reflection_getCallerClass" ? > >> Does the error message occur because of the naming > >> Java_jdk_internal_reflect_Reflection_getCallerClass__ with the added > >> "__" at the end of the method (Reflection.c) ? > > > > That definitely looks like an error in the C source to me. The ending > > "__" should not be there. How is this working ?? > > Ah! It is a remnant of when there were overloads of getCallerClass. The > "__" should be followed by the signature - which is empty in this case. > So IIRC the lookup will first try to use the short name (without the > trailing "__") which will fail, then it will use the long name which > will succeed. > > It should be fixed so we don't waste time doing two lookups. > > David
RE: running java with LD_DEBUG-tracing
Hi David, thanks for clarification . Guess we can just remove the __ now and avoid one lookup. I'll open a bug for this . Best regards, Matthias > On 18/12/2019 10:43 pm, David Holmes wrote: > > On 18/12/2019 7:43 pm, Baesken, Matthias wrote: > >> Hello, I recently worked a bit with the "verbose debugging > >> information" output about operations of the dynamic linker (to > >> sort out some lib loading issues) . > >> See > >> > >> https://docs.oracle.com/cd/E19683-01/816-1386/chapter3-33/index.html > >> http://man7.org/linux/man-pages/man8/ld.so.8.html > >> > >> about the LD_DEBUG flag. > >> > >> > >> By chance I noticed the following output when running a trivial Java > >> program : > >> > >> LD_DEBUG=libs /linuxx86_64/output-jdk- > test/images/jdk/bin/java Test > >> . . . > >> 12241: > >> 12241: > >> /linuxx86_64/output-jdk-test/images/jdk/lib/libjava.so: error: symbol > >> lookup error: undefined symbol: > >> Java_jdk_internal_reflect_Reflection_getCallerClass (fatal) > >> 12241: > >> > >> What is wrong with > >> "Java_jdk_internal_reflect_Reflection_getCallerClass" ? > >> Does the error message occur because of the naming > >> Java_jdk_internal_reflect_Reflection_getCallerClass__ with the added > >> "__" at the end of the method (Reflection.c) ? > > > > That definitely looks like an error in the C source to me. The ending > > "__" should not be there. How is this working ?? > > Ah! It is a remnant of when there were overloads of getCallerClass. The > "__" should be followed by the signature - which is empty in this case. > So IIRC the lookup will first try to use the short name (without the > trailing "__") which will fail, then it will use the long name which > will succeed. > > It should be fixed so we don't waste time doing two lookups. > > David > > > > > David > > > >> > >> > >> Best regards, Matthias > >> > >>
Re: running java with LD_DEBUG-tracing
On 18/12/2019 10:43 pm, David Holmes wrote: On 18/12/2019 7:43 pm, Baesken, Matthias wrote: Hello, I recently worked a bit with the "verbose debugging information" output about operations of the dynamic linker (to sort out some lib loading issues) . See https://docs.oracle.com/cd/E19683-01/816-1386/chapter3-33/index.html http://man7.org/linux/man-pages/man8/ld.so.8.html about the LD_DEBUG flag. By chance I noticed the following output when running a trivial Java program : LD_DEBUG=libs /linuxx86_64/output-jdk-test/images/jdk/bin/java Test . . . 12241: 12241: /linuxx86_64/output-jdk-test/images/jdk/lib/libjava.so: error: symbol lookup error: undefined symbol: Java_jdk_internal_reflect_Reflection_getCallerClass (fatal) 12241: What is wrong with "Java_jdk_internal_reflect_Reflection_getCallerClass" ? Does the error message occur because of the naming Java_jdk_internal_reflect_Reflection_getCallerClass__ with the added "__" at the end of the method (Reflection.c) ? That definitely looks like an error in the C source to me. The ending "__" should not be there. How is this working ?? Ah! It is a remnant of when there were overloads of getCallerClass. The "__" should be followed by the signature - which is empty in this case. So IIRC the lookup will first try to use the short name (without the trailing "__") which will fail, then it will use the long name which will succeed. It should be fixed so we don't waste time doing two lookups. David David Best regards, Matthias
Re: running java with LD_DEBUG-tracing
On 18/12/2019 7:43 pm, Baesken, Matthias wrote: Hello, I recently worked a bit with the "verbose debugging information" output about operations of the dynamic linker (to sort out some lib loading issues) . See https://docs.oracle.com/cd/E19683-01/816-1386/chapter3-33/index.html http://man7.org/linux/man-pages/man8/ld.so.8.html about the LD_DEBUG flag. By chance I noticed the following output when running a trivial Java program : LD_DEBUG=libs /linuxx86_64/output-jdk-test/images/jdk/bin/java Test . . . 12241: 12241: /linuxx86_64/output-jdk-test/images/jdk/lib/libjava.so: error: symbol lookup error: undefined symbol: Java_jdk_internal_reflect_Reflection_getCallerClass (fatal) 12241: What is wrong with "Java_jdk_internal_reflect_Reflection_getCallerClass" ? Does the error message occur because of the naming Java_jdk_internal_reflect_Reflection_getCallerClass__ with the added "__" at the end of the method (Reflection.c) ? That definitely looks like an error in the C source to me. The ending "__" should not be there. How is this working ?? David Best regards, Matthias
running java with LD_DEBUG-tracing
Hello, I recently worked a bit with the "verbose debugging information" output about operations of the dynamic linker (to sort out some lib loading issues) . See https://docs.oracle.com/cd/E19683-01/816-1386/chapter3-33/index.html http://man7.org/linux/man-pages/man8/ld.so.8.html about the LD_DEBUG flag. By chance I noticed the following output when running a trivial Java program : LD_DEBUG=libs /linuxx86_64/output-jdk-test/images/jdk/bin/java Test . . . 12241: 12241: /linuxx86_64/output-jdk-test/images/jdk/lib/libjava.so: error: symbol lookup error: undefined symbol: Java_jdk_internal_reflect_Reflection_getCallerClass (fatal) 12241: What is wrong with "Java_jdk_internal_reflect_Reflection_getCallerClass" ? Does the error message occur because of the naming Java_jdk_internal_reflect_Reflection_getCallerClass__ with the added "__" at the end of the method (Reflection.c) ? Best regards, Matthias
