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

[David Thompson]

> ...
> 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.

That sounds like a good idea.

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

Excellent!

	David

> 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
>>>
>>
>
>
> _______________________________________________
> Paraview-developers mailing list
> Paraview-developers at paraview.org
> http://public.kitware.com/mailman/listinfo/paraview-developers
>