Make location a struct

[dirkschumacher]

This improves efficiency by making the location a struct and also reduce its size (8 fewer bytes). This has likely some effect on the whole search as we consume fewer bytes on an object that gets copied billions of time which decreases the time it takes to check for a valid location (among other things).

The main changes are:

• Make the location a struct to avoid convT allocation unexpectedly somewhere
• Reduce the size by encoding an invalid location as NaN in the float64 fields.

This PR also improves the performance of Unique and Centroid

Benchmarks (only for checking if a location is valid):

name              old time/op    new time/op    delta
Location-8          2.07ns ± 0%    0.79ns ± 0%   -61.72%  (p=0.000 n=9+10)
Unique/n=10-8       3.83µs ± 0%    0.49µs ± 1%   -87.13%  (p=0.000 n=9+10)
Unique/n=100-8      40.3µs ± 0%     4.9µs ± 1%   -87.92%  (p=0.000 n=8+9)
Unique/n=1000-8      418µs ± 0%      47µs ± 3%   -88.79%  (p=0.000 n=10+9)
Unique/n=10000-8    4.28ms ± 0%    0.47ms ± 2%   -89.13%  (p=0.000 n=9+8)
Centroid-8          11.1µs ± 1%     1.9µs ± 0%   -83.25%  (p=0.000 n=10+9)

name              old alloc/op   new alloc/op   delta
Location-8           0.00B          0.00B           ~     (all equal)
Unique/n=10-8       1.99kB ± 0%    0.50kB ± 0%   -74.94%  (p=0.000 n=9+10)
Unique/n=100-8      25.0kB ± 0%     4.7kB ± 0%   -81.22%  (p=0.000 n=10+10)
Unique/n=1000-8      313kB ± 0%      57kB ± 0%   -81.62%  (p=0.000 n=10+10)
Unique/n=10000-8    2.74MB ± 0%    0.49MB ± 0%   -82.28%  (p=0.000 n=9+10)
Centroid-8           24.0B ± 0%      0.0B       -100.00%  (p=0.000 n=10+10)

name              old allocs/op  new allocs/op  delta
Location-8            0.00           0.00           ~     (all equal)
Unique/n=10-8         42.0 ± 0%       4.0 ± 0%   -90.48%  (p=0.000 n=10+10)
Unique/n=100-8         410 ± 0%         7 ± 0%   -98.29%  (p=0.000 n=10+10)
Unique/n=1000-8      4.03k ± 0%     0.01k ± 0%   -99.83%  (p=0.000 n=10+10)
Unique/n=10000-8     40.2k ± 0%      0.0k ± 0%   -99.97%  (p=0.000 n=10+10)
Centroid-8            1.00 ± 0%      0.00       -100.00%  (p=0.000 n=10+10)

In addition most operations on locations can now be inlined.

...
./model_stop.go:142:35: inlining call to common.Location.IsValid
./model_stop.go:142:35: inlining call to common.isValidLongitude
./model_stop.go:142:35: inlining call to common.isValidLatitude
...

In theory this is a breaking change but the API stays the same.

[davidrijsman]

I would not do this. This will allow users to update a Location without us having any control on the values set. Today one can only create a location by invoking the factory method NewLocation which makes it very clear where values are set.

[dirkschumacher]

I would not do this. This will allow users to update a Location without us having any control on the values set. Today one can only create a location by invoking the factory method NewLocation which makes it very clear where values are set.

Users cannot update the fields outside of the nextroute package.

type Location struct {
	longitude float64
	latitude  float64
}

[davidrijsman]

As long as the properties of Location are not exported. Would be nice if this PR is actually also leveraging this change in the nextroute code.

[sebastian-quintero]

🌮

[dirkschumacher]

Would be nice if this PR is actually also leveraging this change in the nextroute code.

@davidrijsman can you elaborate a bit what you mean?

[dirkschumacher]

For now I leave the size of the struct as is. It appears that this is the faster approach during the search as most time is spent on validating locations. We can revisit that later.