Martin Desruisseaux wrote: > Jody Garnett a écrit : >> Reading for an array of ordinates is done way more often; it is okay >> to specify the array returned is not to be edited - but forcing it to >> be a copy every time is a bit painful. > My assumption was that reading would be performed by > > DirectPosition.getOrdinate(i) > > and not > > DirectPosition.getCoordinates()[i] > > The former should not have the copy overhead. I believe that looping > over getOrdinate(int) should be close in performance to looping > directly over array elements in modern JVM. In some implementations > (e.g. an implementation that computes ordinates on the fly using some > algorithm, instead of storing them), getOrdinate(int) may be more > performant than getOrdinates() since the later forces immediate > computation of every coordinates. I agree with your reasoning - I have found lots of client code that just grabs the getCoordinates() so they get a nice comfortable array ... but if they do not read the javadocs they are doomed to poor performance.
So we are agreed the interface is correct - getCoordinates() *must* copy and setOrdinate( index, value ) must be the only way to edit. > The intend was to encourage usage of getOrdinate(int) over > getOrdinates() in order to avoid the performance issue raised. However > I admit that forcing array clone in getOrdinates() is a questionable > decision. The rational was: > > * Precedence given to data integrity over 'getOrdinates()' performance, > especially since I believe that this performance concern can be avoided > with usage of 'getOrdinate(int)' instead (but I admit that I didn't > benchmarked that). > > * A garantee that 'getOrdinates()' always clone the array can help > performance > in some cases. Use case inspired from Geotools code: > > - We want an array of coordinates with the intend to modify it for > computation purpose (without modifying the original DirectPosition), > or we want to protect the array from future DirectPosition changes. > > - If DirectPosition.getOrdinates() is garanteed to not return the > backing > array, then we can work directly on this array. If we don't have > this > garantee, then we must conservatively clone the array in very cases. > > - Cloning the returned array is useless if the implementation > cloned the > array or was forced to returns a new array anyway (for example > because > the coordinates were computed on the fly). Good stuff Martin > An other way to see the idea is: > > * Use DirectPosition.getOrdinates() when you want coordinates in an array > protected from changes in the most efficient way. We can not have as > much > efficiency if DirectPosition.getOrdinates() is not garantee to > returns a > new array on every call. > > * Use DirectPosition.getOrdinate(int) if you want some coordinate values > without the cost of cloning an array. With the trivial > implementation and > using HotSpot compiler, I believe that it is as efficient than > iterating > directly though array elements (but I didn't benchmarked - I just > expect > HotSpot inlining to work as explained on Sun web site). > So I was correct in fixing Sanjay's implementation - I am copying some of your points into the javadocs of DirectPosition. Jody ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ Geotools-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geotools-devel
