<div dir="ltr">Completely shooting from the hip, we have been seeing some slow-downs over the last week or so & there's at least a vague guess that it could be related to these pervasive data structure changes that have been going in within that window.<br><br>Have you been running/seen any compile-time perf changes in that same window?<br><br>One of the things I was thinking, if this is an issue, is that we could do something vtable-esque. Since we already have an enum to differentiate the various derived classes - could we have an array of manual vtables, indexed by that enum, and explicitly invoke through that? If the switch stuff is slowing things down - possible that it isn't, and the switch is cheaper than the indirect call, etc.<br><br>Just some idle thoughts.</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jun 16, 2015 at 4:46 PM, Pete Cooper <span dir="ltr"><<a href="mailto:peter_cooper@apple.com" target="_blank">peter_cooper@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Nick<br>
<br>
As you were the last person to attempt this on PR889, I thought i’d see how you like this series of patches.<br>
<br>
I’ve tried to devirtualize a small number of virtual methods at a time, hence the bunch of patches.  Unfortunately the last one is pretty huge as it handles all the places we used to call delete on Value’s and now need to call a destroy method to dispatch to the correct subclass.  I’m not sure there’s a great way to split the last one up.<br>
<br>
David, i’ve CCed you specifically to ask about how safe some of the changes here are.  In particular, the way i’ve changed things is:<br>
- If a subclass is final (and i’ve added final where I can), then I give it its own delete method which just does ‘using User::operator delete’.  Is this reasonable?  Do i need to handle any other kinds of delete like User does?<br>
- If a subclass is not able to be final, then i’ve added a destroy() method and switch on the value kind to then call ‘delete cast<SubClass>(this)’<br>
<br>
Thanks to Duncan for the tip on std::default_delete, i’ve made use of this on various places which had a std::unique_ptr of say Instruction or Value.  The default deleter for those calls destroy() instead of delete.  Deleter’s aren’t needed for final subclasses because I was able to give them a delete method instead.<br>
<br>
I also extended helpers like DeleteContainerPointers to take a std::default_delete so that we can forward to the destroy methods where needed.<br>
<br>
I measured memory usage on the verify_uselist_order test case Duncan has been using.  This contains debug info and is measured with 'llc ~/verify-uselistorder.lto.opt.bc -o /dev/null --filetype=obj -disable-verify’.  Prior to this change we consumed 814MB, and 800MB after.  So almost 2% savings.  This is in line with what I was expecting as the savings is 8 bytes per Value, but due to malloc alignment on Mac OS, we only save that on an average of half of values.<br>
<br>
Finally, this is the order I did all the work locally.  If any of this makes sense to be squashed or split, please let me know.  In the first patch for example I create Constant.def, but I later change that to Value.def.  This one should obviously be fixed to be Value.def from the start.<br>
<br>
Cheers,<br>
Pete<br>
<br>
</blockquote></div><br></div>