Hi Laurent,
The reason I ask is that it would be against all odds that you'd be able
to detect loading those 2 variables into temp local variables given how
complex a given rendering operation is.
I could imagine that loading a field into a local variable in the inner
loop of the crossings loops might be noticeable, but not for the case of
2 calculations in a once per op setup/teardown method.
I'm not saying that we should never do shadow fields in local variables,
just that we should only do that for cases that affect performance as it
can make the code less readable if it's not going to make a difference
and also you run the slight risk in some cases of updating the local and
not saving it back to the field (usually it's done right when such a
shadow local is introduced, but in some later bug fix some other
engineer decides that they can add a shortcut through the code without
realizing they are short-cutting past a "shadow to field" sync point and
it introduces a bug - thus I reserve a technique like that only for
important cases - in particular loops executed a number of times...)
...jim
On 6/11/15 12:00 AM, Laurent Bourgès wrote:
Jim,
Fixed but I kept the local variable as it has a measurable impact (760ms
vs 800ms) with a medium complex map (135 000 shapes).
Please confirm that I am reading this correctly.
You reverted the 2 variable loads at lines 1258,1259 in Renderer.java and made
no other changes and saw a 5% performance drop on the total runtime of a
benchmark that renders 135,000 shapes?
I only run the benchmark once on each variant and results have some
variability even MapBench has several warmup phases...
However, each map is rendered by 4 threads for 25 iterations on this
test = 3 minutes at 100% cpu on my laptop.
I have the impression it is a bit slower; I will take time to perform 3
runs per variant and compare the best of 3 to have a better estimate.
Laurent
