[Orekit Developers] Frames and WeakHashMap

[Evan Ward <evan.ward@nrl.navy.mil>]

Hi,

First, as this is my first time posting to this list, allow me to 
introduce myself. I'm an engineer working for the US Naval Research Lab 
in astrodynamics. My interests include orbit determination, global 
sensitivity/uncertainty analysis, and ice cream. :)

Thanks to the Orekit team for a well-designed and accurate library.

While looking in to concurrent orbit propagation I noticed several 
issues relating to how Frame and FramesFactory use WeakHashMaps. After 
reading up on WeakHashMap I learned it has two unintuitive attributes:
  - WeakHashMap is *synchronized* on get and put operations. It calls 
getTable() which calls expungeStaleEntries() which is synchronized on a 
final member of the class.
  - WeakHashMap uses WeakReferences to store the *key* and not the value.

In Frame.findCommon(...) use of WeakHashMap causes contention even when 
the common frame is already known because of the synchronized get(..). 
In my highly concurrent test case I noticed a speed up when I simply 
removed the commons cache. My guess is that the logic and comparisons in 
findCommon(...) are insignificant compared to the floating point 
operations in computing the Transform. I propose removing the commons 
map or looking in to using a ConcurrentHashMap (Java 6).

In FramesFactory a WeakHashMap is used to cache frames as they are 
created, with a Predefined as the key. Once created, Frames are never 
removed from the cache because every Predefined is static final (an 
enum). This is not a pressing issue, but it appears that the intent is a 
Frame would be garbage collected once the application is done with it. 
If the current state is acceptable I propose using a plain HashMap to 
avoid the extra overhead. If garbage collecting old Frames is important, 
perhaps a SoftReference as the value would work.

Thanks,
Evan Ward