[prev in list] [next in list] [prev in thread] [next in thread]
List: vtk-developers
Subject: Re: [vtk-developers] Proposal to add "vtkInfoMacro"
From: Berk Geveci <berk.geveci () kitware ! com>
Date: 2014-12-21 22:34:27
Message-ID: CAE32kpWJU_Q_doPv17s9+gT5Lq9znjb8JOGm3e9TH16yp3SY2A () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Thank you Andras. Happy holidays.
-berk
On Sat, Dec 20, 2014 at 8:57 PM, Andras Lasso <lasso@queensu.ca> wrote:
>
> Thanks to all for taking time to consider this. I think in the long term
> such functionality should be part of VTK, but I understand if there are
> other priorities now and I don't want to take more of your time with
> discussing this one particular issue.
>
>
>
> I hope this request will be revisited in the future, please keep it in
> mind when other application support features will be integrated into VTK.
> I've entered a ticket to the issue tracker to make sure it will not be
> forgotten: http://vtk.org/Bug/view.php?id=15220.
>
>
>
> Andras
>
>
>
> *From:* Berk Geveci [mailto:berk.geveci@kitware.com]
> *Sent:* Saturday, December 20, 2014 10:49
>
> *To:* Andras Lasso
> *Cc:* David Cole; David Gobbi; Jean-Christophe Fillion-Robin (
> JChris.FillionR@kitware.com); vtk-developers@vtk.org
> *Subject:* Re: [vtk-developers] Proposal to add "vtkInfoMacro"
>
>
>
> I looked at our current structure and could not find a suitable place that
> would allow for non-GUI classes to have access to this but not the core
> classes. It is probably not worth creating a module to just add this header
> so I recommend keeping it in Slicer for now.
>
>
>
> Best,
>
> -berk
>
>
>
>
>
> On Sat, Dec 13, 2014 at 3:42 PM, Andras Lasso <lasso@queensu.ca> wrote:
>
> You are right, it's not necessary to make this additional logging macro
> available for core VTK library modules. It can be anywhere in VTK where VTK
> tests, examples, and VTK-based applications can access it. Do you have any
> suggestion for a good place?
>
>
>
> Having the macro in VTK allows a standardization on the macro name,
> parameters, and behavior among all VTK-based applications. The macro would
> be used in application classes derived from vtkObject.
>
>
>
> Andras
>
>
>
> *From:* Berk Geveci [mailto:berk.geveci@kitware.com]
> *Sent:* Saturday, December 13, 2014 15:16
> *To:* Andras Lasso
> *Cc:* David Cole; David Gobbi; Jean-Christophe Fillion-Robin (
> JChris.FillionR@kitware.com); vtk-developers@vtk.org
>
>
> *Subject:* Re: [vtk-developers] Proposal to add "vtkInfoMacro"
>
>
>
> Sorry but I remain unconvinced. I haven't heard a compelling reason why
> this functionality should be available to core VTK modules and why the
> maintainers of those modules need to worry about it. So far, the only
> example is for a class in Slicer.
>
>
>
> Whether this is in a separate header or not, putting it in Common/Core
> will make it possible for all modules in VTK to use this functionality
> without any compile errors. As I mentioned, I'd like to see functionality
> migrate out of Common/Core to outer modules not vice versa. "There are
> other application-only functions in Common already" is not a good reason
> why we should cram more in there. Also, vtkOutputWindow is used for
> VTK-level functionality such as debugging and error reporting. Similar to
> vtkDebugLeaks. I wouldn't call that application specific.
>
>
>
> Best,
>
> -berk
>
>
>
> On Sat, Dec 13, 2014 at 11:35 AM, Andras Lasso <lasso@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@aol.com]
> *Sent:* Saturday, December 13, 2014 09:28
> *To:* David Gobbi
> *Cc:* Berk Geveci; vtk-developers@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@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@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@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@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@gmail.com <david.gobbi@gmail.com>]
>
> *Sent:* Friday, December 12, 2014 09:16
> *To:* vtk-developers@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@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@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
>
>
>
>
[Attachment #5 (text/html)]
<div dir="ltr">Thank you Andras. Happy \
holidays.<div><br></div><div>-berk</div></div><div class="gmail_extra"><br><div \
class="gmail_quote">On Sat, Dec 20, 2014 at 8:57 PM, Andras Lasso <span \
dir="ltr"><<a href="mailto:lasso@queensu.ca" \
target="_blank">lasso@queensu.ca</a>></span> wrote:<blockquote class="gmail_quote" \
style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div lang="EN-US" link="blue" vlink="purple">
<div>
<p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">Thanks \
to all for taking time to consider this. I think in the long term such functionality \
should be part of VTK, but I understand if there are other priorities now and I \
don't want to take more of your time with discussing this one particular \
issue.<u></u><u></u></span></p> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"><u></u> \
<u></u></span></p> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">I \
hope this request will be revisited in the future, please keep it in mind when other \
application support features will be integrated into VTK. I've entered a ticket to \
the issue tracker to make sure it will not be forgotten: <a \
href="http://vtk.org/Bug/view.php?id=15220" target="_blank"> \
http://vtk.org/Bug/view.php?id=15220</a>.<u></u><u></u></span></p> <p \
class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"><u></u> \
<u></u></span></p> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">Andras<u></u><u></u></span></p>
<p class="MsoNormal"><a name="14a6a90819a490b8__MailEndCompose"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"><u></u> \
<u></u></span></a></p> <p class="MsoNormal"><b><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif"> Berk Geveci \
[mailto:<a href="mailto:berk.geveci@kitware.com" \
target="_blank">berk.geveci@kitware.com</a>] <br>
<b>Sent:</b> Saturday, December 20, 2014 10:49</span></p><div><div class="h5"><br>
<b>To:</b> Andras Lasso<br>
<b>Cc:</b> David Cole; David Gobbi; Jean-Christophe Fillion-Robin (<a \
href="mailto:JChris.FillionR@kitware.com" \
target="_blank">JChris.FillionR@kitware.com</a>); <a \
href="mailto:vtk-developers@vtk.org" target="_blank">vtk-developers@vtk.org</a><br> \
<b>Subject:</b> Re: [vtk-developers] Proposal to add \
"vtkInfoMacro"<u></u><u></u></div></div><p></p><div><div class="h5"> <p \
class="MsoNormal"><u></u> <u></u></p> <div>
<p class="MsoNormal">I looked at our current structure and could not find a suitable \
place that would allow for non-GUI classes to have access to this but not the core \
classes. It is probably not worth creating a module to just add this header so I \
recommend keeping it in Slicer for now.<u></u><u></u></p>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Best,<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">-berk<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">On Sat, Dec 13, 2014 at 3:42 PM, Andras Lasso <<a \
href="mailto:lasso@queensu.ca" target="_blank">lasso@queensu.ca</a>> \
wrote:<u></u><u></u></p> <blockquote style="border:none;border-left:solid #cccccc \
1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in"> <div>
<div>
<p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">You \
are right, it's not necessary to make this additional logging macro available for \
core VTK library modules. It can be anywhere in VTK where VTK tests, examples, and \
VTK-based applications can access it. Do you have any suggestion for a good \
place?</span><u></u><u></u></p> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> \
</span><u></u><u></u></p> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">Having \
the macro in VTK allows a standardization on the macro name, parameters, and behavior \
among all VTK-based applications. The macro would be used in application classes \
derived from vtkObject.</span><u></u><u></u></p> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> \
</span><u></u><u></u></p> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">Andras</span><u></u><u></u></p>
<p class="MsoNormal"><a \
name="14a6a90819a490b8_14a4563b231e3cdf__MailEndCompose"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> \
</span></a><u></u><u></u></p> <p class="MsoNormal"><b><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif"> Berk Geveci \
[mailto:<a href="mailto:berk.geveci@kitware.com" \
target="_blank">berk.geveci@kitware.com</a>] <br>
<b>Sent:</b> Saturday, December 13, 2014 15:16<br>
<b>To:</b> Andras Lasso<br>
<b>Cc:</b> David Cole; David Gobbi; Jean-Christophe Fillion-Robin (<a \
href="mailto:JChris.FillionR@kitware.com" \
target="_blank">JChris.FillionR@kitware.com</a>); <a \
href="mailto:vtk-developers@vtk.org" \
target="_blank">vtk-developers@vtk.org</a></span><u></u><u></u></p> <div>
<div>
<p class="MsoNormal"><br>
<b>Subject:</b> Re: [vtk-developers] Proposal to add \
"vtkInfoMacro"<u></u><u></u></p> </div>
</div>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal">Sorry but I remain unconvinced. I haven't heard a compelling \
reason why this functionality should be available to core VTK modules and why the \
maintainers of those modules need to worry about it. So far, the only example is for \
a class in Slicer.<u></u><u></u></p> <div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Whether this is in a separate header or not, putting it in \
Common/Core will make it possible for all modules in VTK to use this functionality \
without any compile errors. As I mentioned, I'd like to see functionality \
migrate out of Common/Core to outer modules not vice versa. "There are other \
application-only functions in Common already" is not a good reason why we should \
cram more in there. Also, vtkOutputWindow is used for VTK-level functionality such \
as debugging and error reporting. Similar to vtkDebugLeaks. I wouldn't call that \
application specific.<u></u><u></u></p> </div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Best,<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">-berk<u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal">On Sat, Dec 13, 2014 at 11:35 AM, Andras Lasso <<a \
href="mailto:lasso@queensu.ca" target="_blank">lasso@queensu.ca</a>> \
wrote:<u></u><u></u></p> <blockquote style="border:none;border-left:solid #cccccc \
1.0pt;padding:0in 0in 0in \
6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt"> <div>
<div>
<p class="MsoNormal"><a \
name="14a6a90819a490b8_14a4563b231e3cdf_14a44816fe26603e__MailE"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">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.</span></a><u></u><u></u></p> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> \
</span><u></u><u></u></p> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">What \
do you think about the following?</span><u></u><u></u></p> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> \
</span><u></u><u></u></p> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">* \
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)</span><u></u><u></u></p> <p \
class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> \
</span><u></u><u></u></p> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">* \
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.</span><u></u><u></u></p> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> \
</span><u></u><u></u></p> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">Andras</span><u></u><u></u></p>
<p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> \
</span><u></u><u></u></p> <p class="MsoNormal"><b><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif"> David Cole \
[mailto:<a href="mailto:DLRdave@aol.com" target="_blank">DLRdave@aol.com</a>] <br>
<b>Sent:</b> Saturday, December 13, 2014 09:28<br>
<b>To:</b> David Gobbi<br>
<b>Cc:</b> Berk Geveci; <a href="mailto:vtk-developers@vtk.org" \
target="_blank">vtk-developers@vtk.org</a>; Andras Lasso</span><u></u><u></u></p> \
<div> <div>
<p class="MsoNormal"><br>
<b>Subject:</b> Re: [vtk-developers] Proposal to add \
"vtkInfoMacro"<u></u><u></u></p> </div>
</div>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal">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.<br> <br>
Great to hear talk of testing performance regressions on the dashboard -- looking \
forward to the day that's a reality.<u></u><u></u></p> <div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Thx,<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">David C.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><br>
On Friday, December 12, 2014, David Gobbi <<a href="mailto:david.gobbi@gmail.com" \
target="_blank">david.gobbi@gmail.com</a>> wrote:<u></u><u></u></p> <blockquote \
style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in \
6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt"> <div>
<p class="MsoNormal">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.<u></u><u></u></p> <div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">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.<u></u><u></u></p> </div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> - David<u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal">On Fri, Dec 12, 2014 at 5:12 PM, Berk Geveci <<a \
href="mailto:berk.geveci@kitware.com" target="_blank">berk.geveci@kitware.com</a>> \
wrote:<u></u><u></u></p> <blockquote style="border:none;border-left:solid #cccccc \
1.0pt;padding:0in 0in 0in \
6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt"> <div>
<p class="MsoNormal">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:<u></u><u></u></p> <div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">* Garbage collection<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">* vtkDataArrayTemplate::DataChanged()<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">* Abundant use of Modified()<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">* Progress reporting<u></u><u></u></p>
</div>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Very nice and convenient feature. They may lead to a code \
running more slowly as more threads are added rather than scale up.<u></u><u></u></p> \
</div> <div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">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).<u></u><u></u></p> </div>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">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.<u></u><u></u></p> </div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Best,<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">-berk<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
<div>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal">On Fri, Dec 12, 2014 at 9:52 AM, David Gobbi <<a \
href="mailto:david.gobbi@gmail.com" target="_blank">david.gobbi@gmail.com</a>> \
wrote:<u></u><u></u></p> <blockquote style="border:none;border-left:solid #cccccc \
1.0pt;padding:0in 0in 0in \
6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt"> <div>
<div>
<p class="MsoNormal">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.<u></u><u></u></p> </div>
<div>
<p class="MsoNormal"><span style="color:#888888"> </span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="color:#888888"> - David</span><u></u><u></u></p>
</div>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal">On Fri, Dec 12, 2014 at 7:24 AM, Andras Lasso <<a \
href="mailto:lasso@queensu.ca" target="_blank">lasso@queensu.ca</a>> \
wrote:<u></u><u></u></p> <blockquote style="border:none;border-left:solid #cccccc \
1.0pt;padding:0in 0in 0in \
6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt"> <div>
<div>
<p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">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.</span><u></u><u></u></p> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> \
</span><u></u><u></u></p> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">Adding \
the macro has negligible impact on VTK but would help application \
developers.</span><u></u><u></u></p> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> \
</span><u></u><u></u></p> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">Andras</span><u></u><u></u></p>
<p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> \
</span><u></u><u></u></p> <p class="MsoNormal"><a \
name="14a6a90819a490b8_14a4563b231e3cdf_14a44816fe26603e_14a419"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> \
</span></a><u></u><u></u></p> <p class="MsoNormal"><b><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif"> David Gobbi [<a \
href="mailto:david.gobbi@gmail.com" target="_blank">mailto:david.gobbi@gmail.com</a>] \
<br> <b>Sent:</b> Friday, December 12, 2014 09:16<br>
<b>To:</b> <a href="mailto:vtk-developers@vtk.org" \
target="_blank">vtk-developers@vtk.org</a><br> <b>Cc:</b> Jean-Christophe \
Fillion-Robin; Berk Geveci; Andras Lasso<br> <b>Subject:</b> Re: [vtk-developers] \
Proposal to add "vtkInfoMacro"</span><u></u><u></u></p> <div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<div>
<p class="MsoNormal">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.<u></u><u></u></p> </div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">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.<u></u><u></u></p> </div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Summary: this is a convenience feature, IMHO a mostly harmless \
one, and one that I might use.<u></u><u></u></p> </div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> - David<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal">On Fri, Dec 12, 2014 at 6:27 AM, Berk Geveci <<a \
href="mailto:berk.geveci@kitware.com" target="_blank">berk.geveci@kitware.com</a>> \
wrote:<u></u><u></u></p> <blockquote style="border:none;border-left:solid #cccccc \
1.0pt;padding:0in 0in 0in \
6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt"> <div>
<p class="MsoNormal">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.<u></u><u></u></p>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">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).<u></u><u></u></p> </div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">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.<u></u><u></u></p> </div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Best,<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">-berk<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<div>
<div>
<p class="MsoNormal">On Thu, Dec 11, 2014 at 12:06 PM, Jean-Christophe Fillion-Robin \
<<a href="mailto:jchris.fillionr@kitware.com" \
target="_blank">jchris.fillionr@kitware.com</a>> wrote:<u></u><u></u></p> </div>
</div>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in \
6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt"> <div>
<div>
<div>
<div>
<div>
<div>
<div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt">Hi Folks,
<u></u><u></u></p>
</div>
<p class="MsoNormal" style="margin-bottom:12.0pt">While developing 3D Slicer, we \
created a macro named <br>
<br>
\
vtkInfoMacro<br> <br>
similar to "vtkErrorMacro/vtkDebugMacro/vtkWarningMacro".<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">We would like to contribute it back to VTK \
core.<u></u><u></u></p> </div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"> <u></u><u></u></p>
</div>
<p class="MsoNormal" style="margin-bottom:12.0pt">The associated topic is:
<br>
<br>
<a \
href="http://review.source.kitware.com/#/c/18385/" target="_blank"> \
http://review.source.kitware.com/#/c/18385/</a><u></u><u></u></p> </div>
<p class="MsoNormal" style="margin-bottom:12.0pt">It would be great to get feedback \
before moving forward.<u></u><u></u></p> </div>
<p class="MsoNormal">Thanks<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="color:#888888">Jc</span><u></u><u></u></p>
</div>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</div></div></div>
</div>
</blockquote></div></div>
_______________________________________________
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
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic