GeoTools
  1. GeoTools
  2. GEOT-1092

LookupTableFactory should consider using SoftValueHashMap instead of WeakValueHashMap

    Details

    • Type: Improvement Improvement
    • Status: Closed Closed
    • Priority: Minor Minor
    • Resolution: Won't Fix
    • Affects Version/s: 2.3.1, 2.4-M0, 2.4.0
    • Fix Version/s: 2.4.1
    • Component/s: coverage
    • Labels:
      None

      Description

      LookupTableFactory should consider using SoftValueHashMap instead of WeakValueHashMap in order to achieve better caching of look-up tables. Using A cache based on weak references might result in almost no caching at all depending on how the GC is confgured.

        Issue Links

          Activity

          Hide
          Simone Giannecchini added a comment -
          I have committed a (simple) possible fix for 2.3.x. Please, Martin review it.
          Show
          Simone Giannecchini added a comment - I have committed a (simple) possible fix for 2.3.x. Please, Martin review it.
          Hide
          Martin Desruisseaux added a comment -
          The "FooFactory should consider using SoftValueHashMap instead of WeakValueHashMap" issue come up often. The purpose of WeakValueHashMap is not to cache values. It is to share instances of immutable objects that are already in use somewhere else. A LookupTable can consume up to 256 kilobytes, which is a significant amount of memory. There is a strong advantage to share an instance of LookupTable if such instance is already in use somewhere else, but it is less obvious that we should retain an instance of this object when it is not used any more. A LookupTable is very fast to create. I didn't made any profiling, but my feeling is that retaining 256 kilobytes of memory for no purpose will hurt performances more than the very slight cost of recomputing a new LookupTable when needed. I know that SoftReferences are cleared when memory are needed, put it still put some additional stress on the garbage collector, increase the probability of disk swapping and decrease the memory available for others applications, especially if we start using SoftReferences everywhere with potentially big objects. I think that we should rather use SoftReference for objects that are not too big but costly to compute. LookupTable are in the opposite case.

          In summary, the purpose of WeakHashValueMap in LookupTableFactory is *not* to cache; it is to share existing instances of LookupTable in order to reduce memory usage. I think that WeakReference is appropriate for this goal. Replacing WeakReference by SoftReference seem to defeat this goal to me. But I have no profiling for backing my words, so I may be completely wrong.
          Show
          Martin Desruisseaux added a comment - The "FooFactory should consider using SoftValueHashMap instead of WeakValueHashMap" issue come up often. The purpose of WeakValueHashMap is not to cache values. It is to share instances of immutable objects that are already in use somewhere else. A LookupTable can consume up to 256 kilobytes, which is a significant amount of memory. There is a strong advantage to share an instance of LookupTable if such instance is already in use somewhere else, but it is less obvious that we should retain an instance of this object when it is not used any more. A LookupTable is very fast to create. I didn't made any profiling, but my feeling is that retaining 256 kilobytes of memory for no purpose will hurt performances more than the very slight cost of recomputing a new LookupTable when needed. I know that SoftReferences are cleared when memory are needed, put it still put some additional stress on the garbage collector, increase the probability of disk swapping and decrease the memory available for others applications, especially if we start using SoftReferences everywhere with potentially big objects. I think that we should rather use SoftReference for objects that are not too big but costly to compute. LookupTable are in the opposite case. In summary, the purpose of WeakHashValueMap in LookupTableFactory is *not* to cache; it is to share existing instances of LookupTable in order to reduce memory usage. I think that WeakReference is appropriate for this goal. Replacing WeakReference by SoftReference seem to defeat this goal to me. But I have no profiling for backing my words, so I may be completely wrong.
          Hide
          Simone Giannecchini added a comment -
          I have read and understood your comment. I see your point . My suggestion is only as follows. I have observed that WekReferences get cleared pretty quickly, especially server side hence even if your intention is not to "cache" objects I think that using SoftReferences could be worthwhile. It is worth to note also that softReferences on most JVMs are treated more or less as WeakReferences unless we set an explicit JVM hint (which I do not remember now).

          In the end, this is something we might consider looking into more closely whenever we have sometimes, but in this case we might have left the task there for a while.

          Same thing applies to GEOT-932.
          Show
          Simone Giannecchini added a comment - I have read and understood your comment. I see your point . My suggestion is only as follows. I have observed that WekReferences get cleared pretty quickly, especially server side hence even if your intention is not to "cache" objects I think that using SoftReferences could be worthwhile. It is worth to note also that softReferences on most JVMs are treated more or less as WeakReferences unless we set an explicit JVM hint (which I do not remember now). In the end, this is something we might consider looking into more closely whenever we have sometimes, but in this case we might have left the task there for a while. Same thing applies to GEOT-932 .
          Hide
          Martin Desruisseaux added a comment -
          I realize that my wording may seems rude. It was not my intend and I'm sorry if it looked like that...

          Both WeakReference and SoftReference have usage: we use SoftReference when we want to cache, and WeakReference when we just want to share existing instances without caching (we really want early garbage collection when the instances consume a lot of memory). I would said that whatever we want caching or not depends on the following ratio:

              (time to compute the object) / (memory consumed by the object)

          If this ratio is high, use SoftReference. If this ratio is low, use WeakReference. In the particular case of LookupFactory, I suspect that this ratio is quite low. But I have no benchmark for backing my words, so it is really just an assumption.
          Show
          Martin Desruisseaux added a comment - I realize that my wording may seems rude. It was not my intend and I'm sorry if it looked like that... Both WeakReference and SoftReference have usage: we use SoftReference when we want to cache, and WeakReference when we just want to share existing instances without caching (we really want early garbage collection when the instances consume a lot of memory). I would said that whatever we want caching or not depends on the following ratio:     (time to compute the object) / (memory consumed by the object) If this ratio is high, use SoftReference. If this ratio is low, use WeakReference. In the particular case of LookupFactory, I suspect that this ratio is quite low. But I have no benchmark for backing my words, so it is really just an assumption.
          Hide
          Martin Desruisseaux added a comment -
          The intend for this class was to reduce memory usage by sharing existing instances of potentially large objects. From this point of view, agressive garbage collection is desirable. I recognize the need for caching, but it may be preferrable to perform this caching at a higher level. For example if we cache the RenderedOp images that use a LookupTableJAI object, the later is indirectly preserved from garbage collection. So caching RenderedOp (or yet a higher level: caching GridCoverage2D) indirectly cache the LookupTableJAI.
          Show
          Martin Desruisseaux added a comment - The intend for this class was to reduce memory usage by sharing existing instances of potentially large objects. From this point of view, agressive garbage collection is desirable. I recognize the need for caching, but it may be preferrable to perform this caching at a higher level. For example if we cache the RenderedOp images that use a LookupTableJAI object, the later is indirectly preserved from garbage collection. So caching RenderedOp (or yet a higher level: caching GridCoverage2D) indirectly cache the LookupTableJAI.

            People

            • Assignee:
              Martin Desruisseaux
              Reporter:
              Simone Giannecchini
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: