[vtk-developers] memory bloat, with patch

David Gobbi david.gobbi at gmail.com
Mon Mar 28 15:54:55 EDT 2011


Don't worry about the vector so much... My complaint about it has more
to do with my style preferences than anything, the size=0 check was
the main issue.

It looks like line 544 has a similar issue:
cell->Derivatives(subId, parametricCoord, &values[0], 1, derivative);

 - David



On Mon, Mar 28, 2011 at 1:35 PM, tom fogal <tfogal at sci.utah.edu> wrote:
> Thanks.
>
> The vector would prevent a leak in the case of an exception, but if
> that's not a concern... anyway, done.  I hadn't realized / at least
> forgot the macro made virtual functions, fixed that too.
>
> Anyway, I had tried checking for 0 points before the last revision as
> well: it fixes the crash, but the test still complains about an invalid
> result.  This is the 'GradCellArray' in the test code.
>
> 1272 this time:
>
>  http://review.source.kitware.com/1272
>
> -tom
>
> David Gobbi <david.gobbi at gmail.com> writes:
>> That vtkGradientFilter.cxx code looks buggy to me, it should be
>> doing a check to make sure that there are more than zero points.
>> And it should be using new/delete instead of using an stl vector
>> and then grabbing a pointer to the vector internals.  If you fixed
>> vtkGradientFilter, I think that would be the best solution.
>>
>> A quick note about GetDataDescription: note that in vtkImageData.h it is
>> inlined like so:
>>
>>   int GetDataDescription() { return this->DataDescription; }
>>
>> There is a reason for this.  The vtkGetMacro creates a virtual method,
>> and virtual methods are not inlined.  Considering how liberally these data
>> classes use GetDataDescription() internally, we definitely want to avoid
>> the virtual function call overhead.
>>
>>  - David
>>
>>
>> On Mon, Mar 28, 2011 at 11:56 AM, tom fogal <tfogal at sci.utah.edu> wrote:
>> > David Gobbi <david.gobbi at gmail.com> writes:
>> >> It's a good change, but I found one mistake (noted in the review) so
>> >> it needs re-checking.
>> >
>> > Okay, fixed. =A0Thanks for the review.
>> >
>> >> I did a "git blame" on vtkImageData.cxx, and it turns out that Berk
>> >> did a very similar fix there just a few months ago. =A0You should take
>> >> a look at what he did and keep your work consistent with his. =A0See
>> >> commit 2e68d14f2c59c4163208baf4f935f84034776b6b
>> >
>> > I did this, and uploaded a new version:
>> >
>> > =A0http://review.source.kitware.com/1271
>> >
>> > but I'm now getting a crash with debug iterators in the
>> > TestGradientAndVorticity test. =A0The issue appears to be from
>> > vtkGradientFilter.cxx, near line 111:
>> >
>> > =A0 =A0 =A0vtkCell* Cell =3D Grid->GetCell(Index);
>> > =A0 =A0 =A0double pcoords[3];
>> > =A0 =A0 =A0int subId =3D Cell->GetParametricCenter(pcoords);
>> > =A0 =A0 =A0vtkstd::vector<double> weights(Cell->GetNumberOfPoints());
>> > =A0 =A0 =A0Cell->EvaluateLocation(subId, pcoords, Coords, &weights[0]);
>> >
>> > The 'Cell' that gets returned here is a vtkEmptyCell, the number of
>> > points is 0, and thus the vector is of size 0 and dereferencing it is
>> > illegal.
>> >
>> > It's just fine with master, so I guess it must be returning a different
>> > type in that case. =A0I can't quite see what's gone wrong... could you
>> > (or anyone) take a look?
>> >
>> > -tom
>> >
>> >> On Fri, Mar 25, 2011 at 4:14 PM, tom fogal <tfogal at sci.utah.edu> wrote:
>> >> > One of the devs I work with sent along this patch:
>> >> >
>> >> > =3DA0http://review.source.kitware.com/1259
>> >> >
>> >> > which fortunately still applied to current master. =3DA0Builds fine on
>> >> > Linux, and some spot tests indicate nothing is amiss... plus the diff
>> >> > itself is pretty straightforward. =3DA0This cuts down on our memory us=
>> e in
>> >> > VisIt pretty significantly.
>> >> >
>> >> > Please review || apply.
>> >> >
>> >> > -tom
>> >
>



More information about the vtk-developers mailing list