Re: Draft JEP: Efficient Stack Walking API

[Mandy Chung]

Hi Jeremy,

Thanks for the feedback and the CallerFinder API you have.

On 7/7/2014 9:55 AM, Jeremy Manson wrote:

Hey folks,

I don't know if Mandy's draft JEP has gotten any love,


The JEP process is in transition to 2.0 version.  Hope this JEP will 
come out soon.


but this is something that has (in the past) been a major CPU cycle 
consumer for us, and we've had to invent / reinvent many wheels to fix 
it internally, so we'd love to see a principled solution.


A couple of notes:

- A large percentage of the time, you just want to find one of:
  1) The direct caller of the method,
  2) The first caller outside a given package.



The current thinking is to allow you to find the direct caller as well 
as express the predicate for filtering that will cover these cases.



We added a CallerFinder API that basically looks like this:

// Finds the caller of the invoking method in the current stack that 
isn't in one of the excluded classes

public static StackTraceElement findCaller(Class... excludedClasses);

// Finds the first caller of a given class
public static StackTraceElement findCallerOf(Class... classesToFind);

This isn't the ideal API (it is more the one that happened to be 
convenient when we threw together the class), but it gets the vast 
majority of use cases.




Does it use Thread.getStackTrace() to implement this CallerFinder API?   
Thread.getStackTrace or Throwable.getStackTrace both eagerly capture the 
entire stack trace that is expensive.  We want to have the VM to be able 
to only capture the stack frames that the client needs and the 
implementation as efficient as possible.


2) Even with a super-efficient stack walker, anyone who uses the 
java.util.logging framework pervasively is going to see a lot of CPU 
cycles consumed by determining the caller.


The current LogRecord implementation calls new Throwable that has to pay 
the cost of capturing the entire stack.


 We've had a lot of luck minimizing this by using a bytecode rewriter 
to change callers of log(msg) to log(sourceClass, sourceMethod, msg). 
 This is almost certainly something that could be done (even in a 
principled way!) by the VM; improvements to CPU usage in such apps 
have been dramatic.




Thanks.  I'll make sure to measure and compare the performance with 
java.util.logging using the new stack walk API and also may ask your 
help to determine if you observe the performance difference comparing 
the rewritten bytecode vs the java.util.logging using the new API.


Mandy


Jeremy



On Sun, Mar 30, 2014 at 4:02 PM, Mandy Chung > wrote:


Below is a draft JEP we are considering submitting for JDK 9.

Mandy


Title: Efficient API for Stack Walking

Goal


Define a standard API for stack walking that will be efficient and
performant.

Non-goal


It is not a goal for this API be easy to use via Reflection for
example
use in code that is compiled for an older JDK.

Motivation
--

There is no standard API to obtain information about the caller's
class
and traverse the execution stack in a performant way.  Existing
libraries
and frameworks such as Log4j and Groovy have to resort to using the
JDK internal API `sun.reflect.Reflection.getCallerClass(int depth)`.

This JEP proposes to define a standard API for stack walking that will
be efficient and performant and also enable the implementation up
level the stack walk machinery from the VM to Java and replaces
the current mechanism of `Throwable.fillInStackTrace.

Description
---

There is no standard API to traverse certain frames on the execution
stack efficiently and access the Class instance of each frame.

There are APIs that allow to access the stack trace information:
  - `Throwable.getStackTrace()` and `Thread.getStackTrace()` that
returns
 an array of `StackTraceElement` which contains the classname
 and method name of a stack trace.
  - `SecurityManager.getClassContext()` which is a protected method
 such that only `SecurityManager` subclass can access the class
 context.

These APIs require the VM to eagerly capture a snapshot of the entire
stack trace and returns the information representing the entire stack.
There is no other way to avoid the cost to examine all frames if
the caller is only interested in the top few frames on the stack.
Both `Throwable.getStackTrace()` and `Thread.getStackTrace()` methods
return an array of `StackTraceElement` that contains the classname and
method name of a stack frame but the `Class` instance.

In fact, for applications interested in the entire stack, the
specification
allows VM implementation to omit some frames in the stack for
performance.
In other words, `Thread.getStackTrace()` may return a partial
stack trace.

Re: RFR: [6904367]: (coll) IdentityHashMap is resized before exceeding the expected maximum size

[Ivan Gerasimov]

Thank you Martin for the enhancement!

It's a good idea to get rid of threshold variable!
When the table grows large, it will start to try to resize the table on 
every single put().
Though it shouldn't matter much, as in such situation everything is 
already slow.



On 08.07.2014 4:44, Martin Buchholz wrote:


On Mon, Jul 7, 2014 at 9:41 AM, Martin Buchholz > wrote:


I'd like to offer an alternative version of this change.  webrev
coming soon.


Here's the promised webrev.
http://cr.openjdk.java.net/~martin/webrevs/openjdk9/IdentityHashMap-capacity/ 



- Fixes a typo in the javadoc.
- removes the "threshold" field (WAT, a cache to avoid multiplying by 3?)
- optimizes 3*x into x + x << 1


Unfortunately, x + x << 1 causes the same overflow bug as 3*x:
(int)(1431655766 + 1431655766 << 1) == 2

this can happen in capacity() and, theoretically, in putAll().

I propose to leave the check
 if (expectedMaxSize > MAXIMUM_CAPACITY / 3)
 return MAXIMUM_CAPACITY;
which is overflow resistant.

In putAll, I guess, something like this can be used:
int len = table.length;
if (n > len || n > len - n || n > len - (n << 1))
resize(capacity(n));



- adds more test assertions
- removes the unusual 4/3 slack space for expansion on deserialization.
TODO: the test should be testng-ified, I think.

Re: RFR: [6904367]: (coll) IdentityHashMap is resized before exceeding the expected maximum size

[Ivan Gerasimov]

On 08.07.2014 4:47, Martin Buchholz wrote:

I think this code has an off-by-factor-of-2 bug.

+if (expectedMaxSize > MAXIMUM_CAPACITY / 3)
+return MAXIMUM_CAPACITY;



No, even though it looks like a bug.

(MAXIMUM_CAPACITY / 3) * (3 / 2) == MAXIMUM_CAPACITY / 2.

if expected size > MAXIMUM_CAPACITY / 3,
then the minimum capacity must be > MAXIMUM_CAPACITY / 2
then the minimum capacity == MAXIMUM_CAPACITY.

Re: RFR: 8044656: Update JAX-WS RI integration to latest version

[huizhe wang]

Hi Miran,

In class src/share/jaxws_classes/com/sun/tools/internal/xjc/Options.java:
+// hack to force initialization so catalog manager system 
properties take effect

+staticManager.getVerbosity();

Is that necessary?  I thought the next call to getCatalog would 
initialize the properties from the CatalogManager property file.


Using ServiceLoader in class 
src/share/jaxws_classes/javax/xml/ws/spi/Provider.java:
Would you need to give the process privilege?  Or maybe the method is 
already called in a privileged block?


Thanks,
Joe

On 6/30/2014 9:22 AM, Miroslav Kos wrote:

Hi,
there is a bulk update of JAX-B/WS from upstream projects -
webrev: http://cr.openjdk.java.net/~mkos/8044656/jaxws.00/
more details in issue desc: 
https://bugs.openjdk.java.net/browse/JDK-8044656


Could I ask for a review/approval?

Thanks
Miran

Re: RFR [8035975] Pattern.compile(String, int) fails to throw IllegalArgumentException

[Ivan Gerasimov]

Thanks you Sherman for review!

On 07.07.2014 21:38, Xueming Shen wrote:

On 07/07/2014 10:07 AM, Ivan Gerasimov wrote:

Hello!

The javadoc says that Pattern.compile(String regex, int flag) will 
throw IllegalArgumentException, if flag contains an invalid bit set.

However, it fails to do so.

Would you please help review the simple fix for that?
Alternatively, we could just remove the corresponding @throws part, 
but that would change the spec.


BUGURL: https://bugs.openjdk.java.net/browse/JDK-8035975
WEBREV: http://cr.openjdk.java.net/~igerasim/8035975/0/webrev/

Sincerely yours,
Ivan


Looks fine. But given it's a behavior change, you probably still need 
a CCC for it.



Okay, just filed one.

Sincerely yours,
Ivan


thanks!
-Sherman

Re: Review request: 8029548 (jdeps) use @jdk.Exported to determine supported vs JDK internal API

[Mandy Chung]

On 7/4/14 11:04 AM, Alan Bateman wrote:

On 25/06/2014 21:21, Mandy Chung wrote:

This patch is also intended to target for 8u40 backport.  It fixes
the following issues:

JDK-8029548 (jdeps) use @jdk.Exported to determine supported vs JDK 
internal API

JDK-8039007 jdeps incorrectly reports javax.jnlp as JDK internal APIs
JDK-8031092 jdeps does not recognize --help option.
JDK-8048063 (jdeps) Add filtering capability

Webrev at:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8029548/webrev.00/

I'm skimmed over this and it mostly looks okay. I'm not sure that I 
agree that JDK-8039007 is a bug, this is really just that it is 
missing @jdk.Exported as you noted in PlatformClassPath. 


I think the right way to do is to add @jdk.Exported in javax.jnlp. I'll 
follow up with deploy and check if they can annotate the exported APIs 
in 8u40; if so, I can take this special case out from jdeps.


The -filter options are useful but at the same time I have a concern 
that the number of options to jdeps is growing.


I shared the same concern.  I had the -filter option in my first version 
of jdeps but I didn't include it in the first push to jdeps.   Currently 
jdeps doesn't provide an easy way to find all package-level dependencies 
as it always excludes the dependencies from the same JAR/archive.   Both 
cases are useful for developers in order to understand what APIs a 
library/app is using.  I think -filter option provides a convenient way 
to specify what to be filtered rather than listing the packages in -p or 
-e options.  Do you have any other suggestion to avoid adding -filter 
option?


Mandy

Re: RFR [8035975] Pattern.compile(String, int) fails to throw IllegalArgumentException

[Xueming Shen]

On 07/07/2014 10:07 AM, Ivan Gerasimov wrote:

Hello!

The javadoc says that Pattern.compile(String regex, int flag) will throw 
IllegalArgumentException, if flag contains an invalid bit set.
However, it fails to do so.

Would you please help review the simple fix for that?
Alternatively, we could just remove the corresponding @throws part, but that 
would change the spec.

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8035975
WEBREV: http://cr.openjdk.java.net/~igerasim/8035975/0/webrev/

Sincerely yours,
Ivan


Looks fine. But given it's a behavior change, you probably still need a CCC for 
it.

thanks!
-Sherman

RFR [8035975] Pattern.compile(String, int) fails to throw IllegalArgumentException

[Ivan Gerasimov]

Hello!

The javadoc says that Pattern.compile(String regex, int flag) will throw 
IllegalArgumentException, if flag contains an invalid bit set.

However, it fails to do so.

Would you please help review the simple fix for that?
Alternatively, we could just remove the corresponding @throws part, but 
that would change the spec.


BUGURL: https://bugs.openjdk.java.net/browse/JDK-8035975
WEBREV: http://cr.openjdk.java.net/~igerasim/8035975/0/webrev/

Sincerely yours,
Ivan

Re: RFR 8049220: URL.factory data race

[Paul Sandoz]

On Jul 7, 2014, at 1:07 PM, Peter Levart  wrote:

> Hi Pavel, Alan and Paul,
> 
> Thanks for reviewing. I accepted the suggestions from Pavel and Paul and 
> created webrev.02:
> 
> http://cr.openjdk.java.net/~plevart/jdk9-dev/URL.factory/webrev.02/
> 
> Is this good to go into jdk9-dev?
> 

Looks ok.

Paul.

Re: RFR 8049220: URL.factory data race

[Alan Bateman]

On 07/07/2014 12:07, Peter Levart wrote:

Hi Pavel, Alan and Paul,

Thanks for reviewing. I accepted the suggestions from Pavel and Paul 
and created webrev.02:


http://cr.openjdk.java.net/~plevart/jdk9-dev/URL.factory/webrev.02/

Is this good to go into jdk9-dev?
The comments looks okay to me (and it clear). So I think this is okay to 
push to jdk9/dev.


-Alan

Re: RFR: 8048020 - Regression on java.util.logging.FileHandler

[Daniel Fuchs]

Thanks Alan!

Just for the record the 'final' webrev is here:
http://cr.openjdk.java.net/~dfuchs/webrev_8048020/webrev.02

I will push this shortly...

-- daniel

On 7/4/14 8:24 PM, Alan Bateman wrote:

On 04/07/2014 19:10, Daniel Fuchs wrote:

hmmm. yes - you're right. I should catch that to and break the loop in
that case.
So that would become:

  465 } catch (NoSuchFileException x) {
  466 // Race condition - retry once, and if 
that
  467 // fails again just try the next name in
  468 // the sequence.
  469 continue;
  470 } catch (IOException x) {
  // the file may not be writable for us.
  // try the next name in the sequence
  break;
  }

Thanks for the feedback Alan!
I had missed those cases.

If that is the final change then I think I'm okay (no need to generate a
new webrev). I do think it could be improved a bit more by dropping the
isXXX checks but it's not important.

-Alan

Re: RFR 8049220: URL.factory data race

[Peter Levart]

Hi Pavel, Alan and Paul,

Thanks for reviewing. I accepted the suggestions from Pavel and Paul and 
created webrev.02:


http://cr.openjdk.java.net/~plevart/jdk9-dev/URL.factory/webrev.02/

Is this good to go into jdk9-dev?

Regards, Peter


On 07/04/2014 04:54 PM, Paul Sandoz wrote:

On Jul 3, 2014, at 6:33 PM, Alan Bateman  wrote:

On 03/07/2014 09:43, Peter Levart wrote:

Hi,

I noticed a data race in java.net.URL:

https://bugs.openjdk.java.net/browse/JDK-8049220

I propose the following simple patch:

http://cr.openjdk.java.net/~plevart/jdk9-dev/URL.factory/webrev.01/


A good catch and the change looks good to me.

Yes, well spotted. May i suggest that the following comment is updated:

1109 factory = fac; // volatile write

to say something about ensuring safe publication of a constructed handle? since 
it is often quite tricky to reason about why a volatile write is needed (to 
stamp in, at least, a StoreStore barrier).

For JMM v9 we may not need to mark such a ref as volatile.



I assume it just wasn't noticed because it can only be set once and probably 
rared used too.


Yeah, i wonder whether it would ever get optimized/inlined to the point at 
which re-ordering could practically happen.

Paul.

Re: [8u-dev] Request for approval: 6545422: TEST BUG: NativeErrors.java uses wrong path name in exec

[Ivan Gerasimov]

Thanks Seán!

On 07.07.2014 13:49, Seán Coffey wrote:

Approved.

regards,
Sean.

On 01/07/14 22:32, Ivan Gerasimov wrote:
Corrected the subject line: I'm requesting an approval to push the 
fix into jdk8u-dev repo, not jdk8u20


On 02.07.2014 1:29, Ivan Gerasimov wrote:

Hello!

I'm rerequesting an approval to backport this test fix into jdk8u.
To address the issue with NoClassDefFoundError I added the implicit 
@build line to the test (as it was done in JDK-8043520).


BUGURL: https://bugs.openjdk.java.net/browse/JDK-6545422
WEBREV: http://cr.openjdk.java.net/~igerasim/6545422/0/webrev/
Jdk9 change: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/efeab0eae50f
Review: 
http://mail.openjdk.java.net/pipermail/serviceability-dev/2014-February/014152.html


Would you please approve the backport?

Sincerely yours,
Ivan

On 24.02.2014 13:15, Ivan Gerasimov wrote:


On 24.02.2014 13:03, Alan Bateman wrote:

On 24/02/2014 08:56, Ivan Gerasimov wrote:

Hello!

Would you please approve porting back this test bug fix?
The fix applies cleanly to jdk8u.

Master Bug: https://bugs.openjdk.java.net/browse/JDK-6545422
Jdk9 change: 
http://hg.openjdk.java.net/jdk9/dev/jdk/rev/efeab0eae50f
Review: 
http://mail.openjdk.java.net/pipermail/serviceability-dev/2014-February/014152.html
Ivan - would it be possible to hold back on this one for a bit. 
The updated test is dependent on implicit compilation of the 
classes in the @library and this is causing problems for 
concurrent test runs. The same pattern has slipped into 15-20 
other tests with the result that they all fail intermittently due 
to NoClassDefFoundError issues. I think they should be fixed in 
jdk9-dev but any of these "improved" tests are backported.




Yes, sure!
Actually, I was going to port this fix into jdk6 (the test fails 
there), but modified in such a way that it does not depend on 
testlibrary.
I can still work on jdk6 port without first integrating it into 
jdk8u and jdk7u.


Sincerely yours,
Ivan



-Alan.

Re: [8u-dev] Request for approval: 6545422: TEST BUG: NativeErrors.java uses wrong path name in exec

[Seán Coffey]

Approved.

regards,
Sean.

On 01/07/14 22:32, Ivan Gerasimov wrote:
Corrected the subject line: I'm requesting an approval to push the fix 
into jdk8u-dev repo, not jdk8u20


On 02.07.2014 1:29, Ivan Gerasimov wrote:

Hello!

I'm rerequesting an approval to backport this test fix into jdk8u.
To address the issue with NoClassDefFoundError I added the implicit 
@build line to the test (as it was done in JDK-8043520).


BUGURL: https://bugs.openjdk.java.net/browse/JDK-6545422
WEBREV: http://cr.openjdk.java.net/~igerasim/6545422/0/webrev/
Jdk9 change: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/efeab0eae50f
Review: 
http://mail.openjdk.java.net/pipermail/serviceability-dev/2014-February/014152.html


Would you please approve the backport?

Sincerely yours,
Ivan

On 24.02.2014 13:15, Ivan Gerasimov wrote:


On 24.02.2014 13:03, Alan Bateman wrote:

On 24/02/2014 08:56, Ivan Gerasimov wrote:

Hello!

Would you please approve porting back this test bug fix?
The fix applies cleanly to jdk8u.

Master Bug: https://bugs.openjdk.java.net/browse/JDK-6545422
Jdk9 change: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/efeab0eae50f
Review: 
http://mail.openjdk.java.net/pipermail/serviceability-dev/2014-February/014152.html
Ivan - would it be possible to hold back on this one for a bit. The 
updated test is dependent on implicit compilation of the classes in 
the @library and this is causing problems for concurrent test runs. 
The same pattern has slipped into 15-20 other tests with the result 
that they all fail intermittently due to NoClassDefFoundError 
issues. I think they should be fixed in jdk9-dev but any of these 
"improved" tests are backported.




Yes, sure!
Actually, I was going to port this fix into jdk6 (the test fails 
there), but modified in such a way that it does not depend on 
testlibrary.
I can still work on jdk6 port without first integrating it into 
jdk8u and jdk7u.


Sincerely yours,
Ivan



-Alan.