[vtk-developers] Proposal to add "vtkInfoMacro"

[David Cole]

I agree with Berk. VTK should NOT include the kitchen sink... This belongs
elsewhere -- there are whole separate libraries that are actually focused
on logging, and doing it in a high performance, high functionality manner.
log4cpp, g2log, and boost all come to mind.

Great to hear talk of testing performance regressions on the dashboard --
looking forward to the day that's a reality.

Thx,
David C.


On Friday, December 12, 2014, David Gobbi <david.gobbi at gmail.com> wrote:

> Well, vtkImageReslice is probably not the best "speed" example any more,
> it has become fat with all the features that I've added to it over the
> years :)   And in the old days, a big reason for its speed was that it did
> some bit-twiddling to work around the disastrously slow floor() operations
> on old CPUs.
>
> However, it is certainly true with modern computer architectures, with
> their multiple cores and fast caches, doing _any_ application-level or
> system-level operations within the algorithmic code causes a significant
> performance hit, because that takes the CPU core out of the "zone" where it
> can work at its full potential.
>
>  - David
>
> On Fri, Dec 12, 2014 at 5:12 PM, Berk Geveci <berk.geveci at kitware.com
> <javascript:_e(%7B%7D,'cvml','berk.geveci at kitware.com');>> wrote:
>>
>> I'd be fine with it if the header is put into a higher level module such
>> as GUISupport and has some examples demonstrating its purpose and some
>> documentation providing a guide on how to use it. I would not agree to
>> putting this at a place where it may encourage folks to use it in
>> algorithms and other core functionality. As I mentioned previously, I
>> already have the unpleasant job of ripping out a number of "convenience"
>> functionality that are going to have significant impact on performance as
>> we continue to multi-thread more algorithms. All of these features look
>> harmless when initially added. Then they find their way into core
>> functionality slowly but surely. When you are considering 200-way
>> parallelism (which we are, since we are porting to Intel Xeon Phi already),
>> any little bit makes a difference. Some simple examples:
>>
>> * Garbage collection
>> * vtkDataArrayTemplate::DataChanged()
>> * Abundant use of Modified()
>> * Progress reporting
>>
>> Very nice and convenient feature. They may lead to a code running more
>> slowly as more threads are added rather than scale up.
>>
>> I don't agree that misuse of such features would get caught in reviews.
>> Very few reviewers are conscious of potential performance issues. Because
>> very few developers are performance conscious at this point (Dave Gobbi
>> excepted, he writes amazingly fast code, see image reslice).
>>
>> So this is not the time to pepper core classes with convenience
>> functionality. It is time to simplify and come with very clear guidelines
>> on what core classes and algorithms can and cannot do. Once the development
>> community becomes more aware of these issues and our dashboards are able to
>> catch performance regressions, we can relax some.
>>
>> Best,
>> -berk
>>
>>
>>
>>
>> On Fri, Dec 12, 2014 at 9:52 AM, David Gobbi <david.gobbi at gmail.com
>> <javascript:_e(%7B%7D,'cvml','david.gobbi at gmail.com');>> wrote:
>>>
>>> However, I have reservations about putting this macro in vtkSetGet.h. If
>>> it goes in its own header, then the chances of it being confused with
>>> vtkDebugMacro are significantly reduced.
>>>
>>>  - David
>>>
>>>
>>> On Fri, Dec 12, 2014 at 7:24 AM, Andras Lasso <lasso at queensu.ca
>>> <javascript:_e(%7B%7D,'cvml','lasso at queensu.ca');>> wrote:
>>>
>>>>  I completely agree with David. This is an application support
>>>> feature. There are several examples for such features in VTK, including
>>>> QVTKWidget or vtkOutputWindow::DisplayText(). These are not meant to be
>>>> used in VTK library itself (other than examples and tests) but in VTK-based
>>>> applications. They make the life of application developers easier, allow
>>>> standardization, reduces code/feature duplication.
>>>>
>>>>
>>>>
>>>> Adding the macro has negligible impact on VTK but would help
>>>> application developers.
>>>>
>>>>
>>>>
>>>> Andras
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> *From:* David Gobbi [mailto:david.gobbi at gmail.com
>>>> <javascript:_e(%7B%7D,'cvml','david.gobbi at gmail.com');>]
>>>> *Sent:* Friday, December 12, 2014 09:16
>>>> *To:* vtk-developers at vtk.org
>>>> <javascript:_e(%7B%7D,'cvml','vtk-developers at vtk.org');>
>>>> *Cc:* Jean-Christophe Fillion-Robin; Berk Geveci; Andras Lasso
>>>> *Subject:* Re: [vtk-developers] Proposal to add "vtkInfoMacro"
>>>>
>>>>
>>>>
>>>> On the one hand, I can see that it is useful to have a message macro
>>>> that doesn't require a debug build.  In my own apps, I often write
>>>> app-specific classes that are derived from vtkObject and this is a macro
>>>> that I might use from time to time.  Unlike Berk, I'm not all that worried
>>>> that it might be abused.  For VTK, we have code review.
>>>>
>>>>
>>>>
>>>> On the other hand, I don't think we have a use case for this within VTK
>>>> itself, it would be there purely to serve external VTK classes and apps.
>>>> And people who need this feature could simply put this macro (or a similar
>>>> one) in their own header files.
>>>>
>>>>
>>>>
>>>> Summary: this is a convenience feature, IMHO a mostly harmless one, and
>>>> one that I might use.
>>>>
>>>>
>>>>
>>>>  - David
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> On Fri, Dec 12, 2014 at 6:27 AM, Berk Geveci <berk.geveci at kitware.com
>>>> <javascript:_e(%7B%7D,'cvml','berk.geveci at kitware.com');>> wrote:
>>>>
>>>>  I disagree with this change. There is no compelling VTK specific
>>>> example for why this is needed. The examples are all from Slicer - which to
>>>> me says that they can be implemented in Slicer.
>>>>
>>>>
>>>>
>>>> Adding a logging functionality without clear guidelines on how to use
>>>> it is dangerous. It can lead to folks using it for debugging in performance
>>>> critical sections and since it is not compiled out in Release builds, it
>>>> can lead to significant performance issues, specially in multi-threaded
>>>> code. As it is, there are lot of minor issues like this that we will have
>>>> to go and clean up (progress being one of them).
>>>>
>>>>
>>>>
>>>> The main use case seems to be tracking interaction/workflow changes. My
>>>> suggestion is to employ a method appropriate to that. For example, events
>>>> and listeners.
>>>>
>>>>
>>>>
>>>> Best,
>>>>
>>>> -berk
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> On Thu, Dec 11, 2014 at 12:06 PM, Jean-Christophe Fillion-Robin <
>>>> jchris.fillionr at kitware.com
>>>> <javascript:_e(%7B%7D,'cvml','jchris.fillionr at kitware.com');>> wrote:
>>>>
>>>>     Hi Folks,
>>>>
>>>> While developing 3D Slicer, we created a macro named
>>>>
>>>>                                           vtkInfoMacro
>>>>
>>>> similar to "vtkErrorMacro/vtkDebugMacro/vtkWarningMacro".
>>>>
>>>>   We would like to contribute it back to VTK core.
>>>>
>>>>
>>>>
>>>>  The associated topic is:
>>>>
>>>>                       http://review.source.kitware.com/#/c/18385/
>>>>
>>>>  It would be great to get feedback before moving forward.
>>>>
>>>> Thanks
>>>>
>>>> Jc
>>>>
>>>>
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtk-developers/attachments/20141213/1f2904f3/attachment.html>