Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve ThrowableProxy perfo...

2018-10-05 Thread Carter Kozak
Hi Ralph, I've pushed my benchmark to a branch, it's not in a state that I'd be comfortable merging to master. I had to copy implementations into the benchmark because the java 9 version overshadows the default. Commit is here:

Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve ThrowableProxy perfo...

2018-10-05 Thread Ralph Goers
Carter, Did you check in this benchmark? I posted these numbers on Jigsaw-dev and they would like to see the benchmark. Ralph > On Aug 7, 2018, at 8:10 PM, Carter Kozak wrote: > > My guess is that StackWalker provides additional information beyond > the array of classes > supplied by the

Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve ThrowableProxy perfo...

2018-08-09 Thread Carter Kozak
> I started looking into it but got sidetracked No worries, I've had more time than usual this week to work on side projects. Please don't feel obligated to respond quickly. On Thu, Aug 9, 2018 at 2:15 PM Ralph Goers wrote: > > Looking at your latest commit, your number looks similar to mine. >

Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve ThrowableProxy perfo...

2018-08-09 Thread Ralph Goers
Looking at your latest commit, your number looks similar to mine. Ralph > On Aug 9, 2018, at 11:14 AM, Ralph Goers wrote: > > No, I haven’t committed it. I started looking into it but got sidetracked > with having to earn a living. > > Ralph > >> On Aug 9, 2018, at 11:13 AM, Carter Kozak

Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve ThrowableProxy perfo...

2018-08-09 Thread Ralph Goers
No, I haven’t committed it. I started looking into it but got sidetracked with having to earn a living. Ralph > On Aug 9, 2018, at 11:13 AM, Carter Kozak wrote: > > I've pushed the benchmark here: > https://github.com/apache/logging-log4j2/pull/209 > > Is the extended stack trace benchmark

Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve ThrowableProxy perfo...

2018-08-09 Thread Ralph Goers
I created a micro benchmark for the extended stack trace. It isn’t pretty. Ralph > On Aug 9, 2018, at 10:53 AM, Carter Kozak wrote: > > That's true, as long as we collect the stack trace when the > ThrowableProxy is created, the rest can be deferred. I can take a look > at this. > > I'm

Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve ThrowableProxy perfo...

2018-08-09 Thread Carter Kozak
That's true, as long as we collect the stack trace when the ThrowableProxy is created, the rest can be deferred. I can take a look at this. I'm working on a benchmark with a larger stack to simulate conditions in a real application as well. -ck On Thu, Aug 9, 2018 at 12:47 PM, Ralph Goers

Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve ThrowableProxy perfo...

2018-08-09 Thread Ralph Goers
What I don’t understand is that ThrowableProxy seems to be doing a whole bunch of stuff in the constructor. I would think a lot of that could be deferred until the Throwable is being rendered. Ralph > On Aug 9, 2018, at 8:10 AM, Carter Kozak wrote: > > The piece I'd like to avoid is

Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve ThrowableProxy perfo...

2018-08-09 Thread Carter Kozak
The piece I'd like to avoid is ThrowableProxy.loadClass invocations, which can be expensive depending on the ClassLoader implementation. The ThrowablePatternConverter implementations all fall back to the default implementation which operates directly on a Throwable rather than the proxy object. I

Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve ThrowableProxy perfo...

2018-08-08 Thread Ralph Goers
What is the purpose of disabling ThrowableProxy? All the throwable pattern converters rely on it. Ralph > On Aug 8, 2018, at 8:34 AM, Carter Kozak wrote: > > I'm curious what you would think of a system property to disable > ThrowableProxy creation? > My initial preference was to avoid this

Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve ThrowableProxy perfo...

2018-08-08 Thread Ralph Goers
Yup. Part of the problem is that StackWalker is optimized for only walking a few frames. I think it caches 8 stack frames. After that it apparently slows down a lot. I don’t recall there being any way to tune that. Ralph > On Aug 8, 2018, at 5:07 PM, Remko Popma wrote: > > What I remember

Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve ThrowableProxy perfo...

2018-08-08 Thread Remko Popma
What I remember from email interactions that Ralph had with the people working on the Stackwalker implementation: Ostensibly Stackwalker was supposed to give performance benefits for use cases like ours. They even mentioned discovering the location of a logging call as one of the main use

Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve ThrowableProxy perfo...

2018-08-08 Thread Matt Sicker
StackWalker seems to be implemented purely as it says: walking the call stack. If all you need is the stack of classes (e.g., for checking runtime permissions based on the calling class), then the SecurityManager makes most sense since it seems built for that exact purpose. I'd imagine there are

Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve ThrowableProxy perfo...

2018-08-07 Thread Carter Kozak
My guess is that StackWalker provides additional information beyond the array of classes supplied by the SecurityManager, though I've not done a thorough analysis yet. A quick targeted benchmark of our current implementations: Benchmark Mode Cnt

Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve ThrowableProxy perfo...

2018-08-07 Thread Ralph Goers
I have to wonder why using the security manager is faster than using StackWalker. StackWalker was created just for this purpose. Is the way it is implemented the problem? Ralph > On Aug 7, 2018, at 1:34 PM, cakofony wrote: > > GitHub user cakofony opened a pull request: > >