<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Apr 26, 2016 at 2:58 PM, Sean McBride <span dir="ltr"><<a href="mailto:sean@rogue-research.com" target="_blank">sean@rogue-research.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi all,<br>
<br>
So we're continuing to profile our app to improve the performance of something, and I have another vtkDataArray question.  This method:<br>
<br>
void vtkDataArray::SetTuple(vtkIdType dstTupleIdx, vtkIdType srcTupleIdx,<br>
                            vtkAbstractArray *source)<br>
<br>
starts with 3 checks.  If they fail, it uses vtkErrorMacro and bails.<br>
<br>
First, what are the semantics of hitting vtkErrorMacro?  Is it meant to be like an assertion, ie something that should be impossible, and therefore useful in debug but discardable in release?<br></blockquote><div><br></div><div>I've often wondered this myself, I'm not aware of an 'official' interpretation of these. From context, it seems to usually be used to enforce a documented/logical constraint. I often use them as non-fatal asserts.</div><div><br></div><div>I know the dashboards are configured to treat any output coming from vtkErrorMacro as a test failure.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
We've added assert(0) in the three branches, and our code never hits them and no VTK unit test does either.<br></blockquote><div><br></div><div>Not surprising -- the method does nothing (and tests will fail) if it hits them :)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
The 3 checks are individually fast, but the method is called so very very often that removing the 3 checks entirely gives us a full 20% runtime speedup of Render(), so I'm hoping it would be acceptable to wrap them in #ifndef NDEBUG, or otherwise remove them from release builds.</blockquote><div><br></div><div>+1 to the idea. I'd love to see this wrapped up in a vtkAssertMacro or similar, as well as some clarification on the semantics of Warning/ErrorMacro. I think there was some discussion started by Kyle Lutz a few years back about whether or not to just use asserts in these situations, and there was some controversy around the idea and he abandoned the proposal in the end. I can't remember or imagine why people objected at the moment, but the thread should be in the archives.</div><div><br></div><div>I'd actually prefer to remove that (and similar) methods completely (settle down folks, I'm not actually proposing this! I've learned to just accept these warts and not try to fix problematic APIs in VTK). They have performance problems beyond the sanity checks, the repeated dispatch calls add up, too. It's too fine-grained an action for the expense it occurs. It used to be worse, actually -- back when I was profiling some of these functions the vast majority of CPU time was spent in strcmp, because SafeDownCast was being used to test if the source arrays was a data array. I believe it was actually this method that made me add the vtkAbstractArray::FastDownCast system.</div><div><br></div><div>Dave</div></div></div></div>