Instantiation of new Throwable in TxConnectionListener.TransactionSynchronization

[lbarreiro]

Hi,

During a performance evaluation to an high load application, one thing that showed up in the profile data was the initialization of org.jboss.jca.core.connectionmanager.listener.TxConnectionListener.TransactionSynchronization class.

The current workload generates 8000 of these objects each second. With each one of them there is an new Throwable being instantiated (from the records available, it never gets thrown). The cost of instantiating a Throwable object is far greater than throwing it or instantiate a regular object due to the way the JVM collects the stack trace.

For our use case, the best is that the Throwable is only instantiated if it is to be thrown. Looking at the source code, it's possible to do that. Here is my naive attempt to do it.

While I understand that the way the code works now can give useful debug information in some multi-thread environments, it can also be misleading as you are providing a stack trace that it's not related to the current code being executed. If that info is considered important I would suggest that in that case record other information (like the thread name, for example) instead of the full stack trace.

Some more free CPU cycles would certainly help in the quest of running this application at an even higher load.

Running ironjacamar-1.0.20.Final-redhat-1

Thanks in advance,

[jesper.pedersen]

It needs to be the allocation trace that needs to be recorded, so the code is correct.

However, if you can prove that it impacts your load I'm open to adding a system property that switches it off, e.g. null'ing out the value of the exception. You can move the code for this to the constructor.

The property could be called: ironjacamar.disable_enlistment_trace.

See a similar patch for JBJCA-1107 on how to do it. Note, that all patches needs to be submitted against the 1.1 branch.

[stalep]

the cost of creating a Throwable is fairly known since it does a call to  fillInStackTrace() which fills in the execution stack trace. this is not "free". we can probably give you numbers, but frankly i cant see why you should need more convincing regarding this? we would never bother using time on this if it wasnt an overhead....

[stalep]

norman maurer wrote a very good blog post regarding this issue today, read it here for numbers and more info: http://normanmaurer.me/blog/2013/11/09/The-hidden-performance-costs-of-instantiating-Throwables/

[jesper.pedersen]

It isn't a matter of performance, it is a matter of being able to do support for faulty apps - the trace is needed in order to track faulty enlistment scenarios, so it isn't an option to remove it. I have looked.

I have said I'll consider a patch to turn it off for applications that are known to behave correctly.

[stalep]

in this context it has everything to do with performance and to make it clear, i understand why its there (the Throwable) in the first place, i get that. what i find strange is that you're reluctant to change it when the reason is so obvious. the initial commit luis created shouldnt change the stack trace by much?

- but if its the only way you'll accept it we'll make the change with using a property.

[jesper.pedersen]

Look at the comments in the code around it -- it aims to find separate threads doing the work, so the actual enlist() needs to capture the original thread.

The change is noted, but it is likely faster for you to provide the contribution along with any data you have.

[andy.miller]

May I suggest that we handle this two ways.  Upstream, we should not be relying on a system property, but it should be put into the domain model as a configurable option.  The system property is okay for 1.0.x which would make its way into EAP 6.x, but we will also have to document it for both.