Make Trace instances as static final to give JIT a chance to optimize #156
Conversation
| private void initTrace(){ | ||
| trace = TraceFactory.getTraceFactory().getTrace(StoreableCachingMap.class); | ||
| } | ||
| private static final Trace trace = TraceFactory.getTraceFactory().getTrace(StoreableCachingMap.class); |
There was a problem hiding this comment.
I am fine with the other changes. Here, I am not sure if @aclement had anything special in mind by choosing a transient instance field rather than a static one. Andy, I know 2012 was long ago. Can you remember anything in the context of Bugzilla 386341? If you are OK with this particular change here, I would merge it.
There was a problem hiding this comment.
Would be ok for merge if I revert this one?
There was a problem hiding this comment.
@turbanoff, I am reaching out to tell you that I am not ignoring your PRs because I do not like them or so. I simply would prefer Andy to review and merge them. But currently being the only active maintainer and busy with my normal job as well, I simply prioritise scratching my own itches, fixing bugs or working on little improvements of my personal interest. I am sure your improvements are valuable, but assessing whether they might have undesired side effects in parts of the code I do not know is simply time I currently choose to spend elsewhere, be it on AspectJ or privately. But your work is not lost. Please do not feel discouraged to open PRs. This is about my own situation, not about the quality of your contributions. Please accept my apologies. 🙂
It's known that JIT could speculate about classes fields.
JIT in HotSpot require such fields to be marked as
static final. https://shipilev.net/jvm/anatomy-quarks/15-just-in-time-constants/I propose to mark all
Traceinstances asstatic final, as they are used very often.I will update other fields in following PRs. Focused here only one class to minimize changes and make review easier.