Allocations optimization for converters

[charlesbr1]

trafficstars

Made use of cached string values & thread local buffers to avoid frequent allocations.

[chrjohn]

Thanks for the PR (or actually the PRs ;)) Just a question regarding the ThreadLocal Context objects. Why are you using StringBuffer instead of StringBuilder? Due to the ThreadLocal implementation only one thread should access it, shouldn't it? Thanks, Chris.

[charlesbr1]

Hello,

Regarding QuickFix, I may have a fix to commit because of a regression on my changes, regarding double to String conversion, for DoubleField. May be, I have to check what kind of code I gave you for this specific part because the DoubleField optimization is the only piece that I didn’t took from the code I wrote in my work... In prod I made a mistake and we converted double values to string using String.valueOf(), but a NumberFormat must be used instead. Second point is to put some new parameters in conf’, as you said, for size and number of buffers to use with the mina sending thread, but I didn’t took time to look how to do it, for the moment.

For you question, I obviously know the difference between StringBuffer / Builder, I use the Builder when I can. Where did you see that I use a Buffer ? If I did it there is likely a reason, certainly the code below is not compliant with Builder but only Buffer. This is the case for DateTIme formatting, from what I remember. We can avoid one allocation by calling this kind of method, which is only Buffer compliant :

public abstract StringBuffer format(Date date, StringBuffer toAppendTo, FieldPosition fieldPosition);

There is something you have to know with my code there, the FieldPosition argument give my difficulties. I wanted to give the same one by default as the JDK do, but this is not an instance accessible without introspection. And we cannot build the same one as the JDK do because of accessibility restrictions. But the one I one use seems to be working fine :

private static final class DontCareFieldPosition extends FieldPosition { // The singleton of DontCareFieldPosition. static final FieldPosition INSTANCE = new DontCareFieldPosition();

private DontCareFieldPosition() {
    super(0);
}

} If you know this aspect of Java…

Regards,

Charles Briquel

Le 12 déc. 2015 à 18:33, Christoph John [email protected] a écrit :

Thanks for the PR (or actually the PRs ;)) Just a question regarding the ThreadLocal Context objects. Why are you using StringBuffer instead of StringBuilder? Due to the ThreadLocal implementation only one thread should access it, shouldn't it? Thanks, Chris.

— Reply to this email directly or view it on GitHub https://github.com/quickfix-j/quickfixj/pull/34#issuecomment-164170468.

[charlesbr1]

We can use reflexion to resolve Builder issue and avoid a copy of date between StringBuffer - StringBuilder in DateConverters 'public static void convert(Date d, StringBuilder stringBuilder)' methods. Without having to rewrite the JDK or depends on an external lib. But it is not justified there. Furthermore, with Lock Coarsening StringBuffer sync overhead should be removed, but it's still better to drop the potential glitch at the source. So I think it's ok like that.