search

amelroua / osmdroid

Automatically exported from code.google.com/p/osmdroid
0 stars 0 forks source link

GeoPoint perf improvement: multiply by 1E-6 #512

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
GeoPoint conversions from internal int representation of lat/lon to double 
imply a division by 1E6. 
It is more efficient to multiply by 1E-6: from my experiments, multiplying is 
twice faster than dividing. 

The 2 functions for which this improvement seems relevant are: 
GeoPoint.getLongitude() and GeoPoint.getLatitude() - as they can be called 
massively when handling objects with a lot of GeoPoints.

Original issue reported on code.google.com by mathieu....@gmail.com on 14 Jan 2014 at 9:58

GoogleCodeExporter commented 9 years ago 
This question also has some nice answers:
http://stackoverflow.com/questions/226465/should-i-use-multiplication-or-divisio
n
Always use whatever is the clearest.
Multiplication is faster, division is more accurate.

Original comment by neilboyd on 16 Jan 2014 at 8:29

GoogleCodeExporter commented 9 years ago 
Hi Neil, 

If you really didn't care about optimizations, osmdroid would be much much 
simpler: no file caching, no memory caching, no threads,... And it would also 
be useless. 
Look at osmdroid PathOverlay.draw method: this is fine tuned for performances 
(which is important for drawing long paths) - and it is also a particularly 
unclear piece of code... 

I fully agree not to replace "/2" by "*0.5" (as "/2" is clearest in most 
cases). 
But franckly, in our case, I don't see why "/1E6" would be clearest than 
"*1E-6". 
And getLongitude and getLatitude are 2 basic operations when using GeoPoint, so 
easily doubling efficiency here is not irrelevant. 

About difference in accuracy: urban legend, we are using doubles. 

Anyway, this whole subject is a minor point, with easy workarounds. I don't 
want to annoy you on it, do as you prefer. 

Best regards, and thanks for the job!

Original comment by mathieu....@gmail.com on 18 Jan 2014 at 1:05

GoogleCodeExporter commented 9 years ago 
Okay! I wasn't trying to speak against the idea, I was just reporting the 
results of a quick bit of googling. I'll do this change.

Original comment by neilboyd on 18 Jan 2014 at 3:17

GoogleCodeExporter commented 9 years ago 
I made some changes in revision 1400.

Original comment by neilboyd on 18 Jan 2014 at 9:02

GoogleCodeExporter commented 9 years ago 
Are we sure that Java doesn't optimize this for us already at compile or 
runtime?

Original comment by kurtzm...@gmail.com on 20 Jan 2014 at 3:00

GoogleCodeExporter commented 9 years ago 
I wrote a simple test in revision 1416. I didn't see any difference between 
multiply and divide.

Original comment by neilboyd on 20 Jan 2014 at 11:27

GoogleCodeExporter commented 9 years ago 
In the code inside your loop: 
  long l = r.nextLong();
  double s = l / 1E6;
most of the time is probably spent in r.nextLong(), making the difference on 
the second line negligible. 

Here is my test:

void test1(){
  double delta = 0.0;
  for (int i=0; i<10000000; i++){
    delta = i*1E-6; 
  }
}

void test2(){
  double delta = 0.0;
  for (int i=0; i<10000000; i++){
    delta = i/1E6; 
  }
}

On Android 2.2 and 4.3 AVD, test1 is twice faster than test2. 
So, to answer to Kurtz: yes, we are sure that Java doesn't optimize this for 
us.

Original comment by mathieu....@gmail.com on 23 Jan 2014 at 9:50

GoogleCodeExporter commented 9 years ago 
Now I can see that for 1E7 operations you can save about 500ms on my slow 2.3.6 
device.
I imagine that if you're doing anything this many times then there will be 
worse things to worry about than this.
But anyway I already made the change as I said in comment 4.

Original comment by neilboyd on 23 Jan 2014 at 10:38

GoogleCodeExporter commented 9 years ago

Original comment by neilboyd on 25 Jan 2014 at 6:09