[vtk-developers] Proposal to add "vtkInfoMacro"

Sat Dec 13 13:46:51 EST 2014

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