[vtk-developers] Proposal to add "vtkInfoMacro"

Sat Dec 13 15:53:29 EST 2014

Slicer uses the CTK logger, which collects log messages from VTK, ITK, Qt, etc. What we miss is a level below, a standard generic (non-error) VTK macro that collects the message, line number, file name, etc. and sends it to the logger.

Andras

-----Original Message-----
From: Bill Lorensen [mailto:bill.lorensen at gmail.com] 
Sent: Saturday, December 13, 2014 15:17
To: Andras Lasso
Cc: David Cole; David Gobbi; vtk-developers at vtk.org; Berk Geveci
Subject: Re: [vtk-developers] Proposal to add "vtkInfoMacro"

But, Slicer uses ITK, so why not use its logger.

On Sat, Dec 13, 2014 at 2:54 PM, Andras Lasso <lasso at queensu.ca> wrote:
> Thanks for the pointer, Bill. The logger in ITK is quite nice and the 
> info level logging macro is available in ITK: if the output window is 
> set to itkLoggerOutput then
>
> itkGenericOutputMacro
>
> calls
>
> this->m_Logger->Write(LoggerBase::INFO, t)
>
> I don't think that adding an ITK-like logger to VTK is necessary, just adding the missing vtkInfo (or vtkGenericOutput) macro would be enough.
>
> Andras
>
>
> -----Original Message-----
> From: Bill Lorensen [mailto:bill.lorensen at gmail.com]
> Sent: Saturday, December 13, 2014 13:47
> To: Andras Lasso
> Cc: David Cole; David Gobbi; vtk-developers at vtk.org; Berk Geveci
> Subject: Re: [vtk-developers] Proposal to add "vtkInfoMacro"
>
> ITK has a logger, itk::Logger.
>
>
> On Sat, Dec 13, 2014 at 11:35 AM, Andras Lasso <lasso at queensu.ca> wrote:
>> Yes, sure we don’t need VTK to include a complete logging framework 
>> such as log4cpp. We just need a convenient mechanism to send messages 
>> to logging frameworks from VTK.
>>
>>
>>
>> What do you think about the following?
>>
>>
>>
>> * Allow completely disable vtkInfoMacro by an #ifdef (for example, if 
>> #ifndef VTK_INFO_ENABLED then vtkInfoMacro is an empty statement; we 
>> can set VTK_INFO_ENABLED to false by default)
>>
>>
>>
>> * Put the macro in a separate header in Common/vtkInfo.h to not make 
>> the macro visible to VTK classes by default. GUISupport directory 
>> would be too restrictive, as logging is not related to GUI and there 
>> are other application-only functions in Common already (such as 
>> vtkOutputWindow::DisplayText), but any other suggestion is welcome.
>>
>>
>>
>> Andras
>>
>>
>>
>> From: David Cole [mailto:DLRdave at aol.com]
>> Sent: Saturday, December 13, 2014 09:28
>> To: David Gobbi
>> Cc: Berk Geveci; vtk-developers at vtk.org; Andras Lasso
>>
>>
>> Subject: Re: [vtk-developers] Proposal to add "vtkInfoMacro"
>>
>>
>>
>> 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>
>> 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> 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> 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]
>> Sent: Friday, December 12, 2014 09:16
>> To: 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>
>> 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> 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
>>
>>
>>
>>
>> _______________________________________________
>> Powered by www.kitware.com
>>
>> Visit other Kitware open-source projects at 
>> http://www.kitware.com/opensource/opensource.html
>>
>> Follow this link to subscribe/unsubscribe:
>> http://public.kitware.com/mailman/listinfo/vtk-developers
>>
>>
>
>
>
> --
> Unpaid intern in BillsBasement at noware dot com

--
Unpaid intern in BillsBasement at noware dot com