[vtk-developers] [Paraview-developers] change to vtkExodusIIReaderPrivate::ResetCache()

[Fabian, Nathan]

Looking back, my email must have been written at the end of the day... It
doesn't make as much sense as I thought it did :)

When I find some time, I will go back through and add Register/UnRegister
to the code as a helpful indicator to future modifiers that they should
always do that too.

Also, I strongly agree that the cache size should be exposed to the
paraview inspector.  Maybe even strongly enough to pose a patch :)

On 4/27/11 10:14 AM, "David Thompson" <dcthomp at sandia.gov> wrote:

>> I just found an issue where the polygon code was assuming the cache
>> was
>> valid after a second call (which destroyed the array from the previous
>> call).
>>
>> I added Register/Unregister to keep the array.  Which may not really
>> be
>> any safer.
>
>That is exactly what should be done.
>
>> What is the proper response?  Call out to GetCacheOrRead each
>> time an array is used?
>
>What do you mean by "call out to GetCacheOrRead"? That is how the
>array was obtained was it not? The cache should hold on to it until
>the next call to GetCacheOrRead, at which point it *may* release its
>reference. If someone else holds a reference at that point then the
>object stays around. Otherwise, kaput!
>
>> I haven't really carefully audited the code, but in general it does
>> look
>> like GetCacheOrRead is called once and then that array is used
>> immediately
>> after, but I could see this cropping up again...
>
>Yes. I tried to document GetCacheOrRead as best I could, but there is
>that danger. I think the more insidious (or at least more difficult to
>detect) problem would be where a function calls another function which
>in turn calls GetCacheOrRead. That is why you should always
>immediately Register() any object returned by the cache until you're
>done with it.
>
>    D
>
>>
>> On 4/25/11 12:21 PM, "David Thompson" <dcthomp at sandia.gov> wrote:
>>
>>>> From a pull from master some weeks ago, the previous version of this
>>>> method looked like this:
>>>> ...
>>>> When reading in an exodus file now, the code disappears into the
>>>> reader and never finishes (maybe it would eventually, but after a
>>>> long while I kill it).  Adding back in the line to set the cache
>>>> capacity to 128 resolves this issue.  I expect that the line was
>>>> removed for a reason.  Did removing the line resolve a problem for
>>>> someone else?  Is anyone else having difficulty with the exodus
>>>> reader as a result of this change?  Or is it something specific to
>>>> my exodus file/runtime environment?
>>>
>>> Hi Pat,
>>>
>>> The line was removed to address a problem Alan had reported when
>>> opening many small files with the parallel reader... the sum of the
>>> caches (one from each sub-reader owned by the parallel reader) was
>>> growing too large. I haven't heard whether anyone else is having
>>> problems but Alan reported it fixed his issue. If I had to guess, I
>>> would say there's some code in there relying on the cache holding on
>>> to a reference to a vtkDataArray when it should not (even with a
>>> 128MiB cache, there was never any guarantee). If so, valgrind should
>>> pick up on the reference to freed memory. Can you run your code
>>> through valgrind and see if it complains?
>>>
>>>   David
>>>
>>>
>>> _______________________________________________
>>> Paraview-developers mailing list
>>> Paraview-developers at paraview.org
>>> http://public.kitware.com/mailman/listinfo/paraview-developers
>>>
>>
>>
>> _______________________________________________
>> Paraview-developers mailing list
>> Paraview-developers at paraview.org
>> http://public.kitware.com/mailman/listinfo/paraview-developers
>>
>