6 ms·
LTO can be particularly handy when doing devirtualization. Our experience ([1]) with Chromium demonstrated that there's a fair amount of virtual methods which h
by krasin 8y ago
LTO can be particularly handy when doing devirtualization. Our experience ([1]) with Chromium demonstrated that there's a fair amount of virtual methods which have exactly two implementation in the code: one for production and another one for tests. While linking production code, it's trivial for this optimization to spot that, replace a virtual call with regular invocation and then often inline and allow for other in-place optimizations. In many cases of the renderer (Blink), that gives 3%-7% speedup out of nowhere.
1. https://bugs.chromium.org/p/chromium/issues/detail?id=580389#c25 https://bugs.chromium.org/p/chromium/issues/detail?id=580389...
- simcop2387 8y agoThat's really cool. I've primarily seen LTO benefits on the more embedded side of things where it ends up enabling far more aggressive dead code elimination between code and libraries which makes it much easier to fit into small chips. I hadn't thought much about virtual methods with C++ code (uncommon to see those in embedded code to begin with).
- mehrdadn 8y agoOut of curiosity, why use virtual calls in those cases? Why not just have two different implementation files -- one for tests, and one for production -- and choose one based on which one you're doing? It seems like a win all around.
- fulafel 8y agoThere's always performance hacks to be found in big apps, in exchange for implementation complexity and man-hours. And there's always a next bottleneck to be found. Performnace bugs are subject to an extended form of survivorship bias where they become bigger when the preceding perf bottlenecks get fixed.
- mehrdadn 8y agoI've refreshed and seen three major revisions of your comment after writing a full reply to the first one, and I can't say I've been able to clearly follow where you're coming from and where you're going with any of them. I would suggest that if you feel the need for such drastic revisions, it may be worth reconsidering whether whatever you're trying to argue is really compelling.
- slededit 8y agoHis advice is sage and you should look past whatever deficiencies you find in his form of communication. Simply put there's always bigger fish to fry, and virtual methods are a quick and easy way to implement test stubs. Especially when the compiler devirtualizes.
- mehrdadn 8y agoWell I had a response to that originally but his edits kind of changed how much sense my reply made in response. Here's what I had: Performance is only one aspect of it. It also reduces code bloat, reducing the program's size footprint. Most tests (yes, I know, not all, but most) should not make it into the final binary users are running. I also don't see what's "hacky" about making a foo.test.cc file when I want an alternate implementation for foo.cc. It seems to be quite a positive and clear way to document the fact that an implementation is only needed for testing, and vice-versa. And not only that, but it reduces compile (& link) times, since you only need to compile one of the two implementations for each use case.
- slededit 8y agoYea all things being equal I'd choose your approach. But in an existing project that did things this way - it would be the last thing I'd touch. This is of course barring extreme circumstances - which profiling and other tools would identify. Unfortunately a lot of C++ developers don't really understand how the linker works, and because of that the build system is black magic.
- Matthias247 8y agoFrom my point of view a benefit for virtual calls is toolability. If you have an abstract interface, it’s pretty easy to either directly create mocks for it in the test file, or use sth like gmock. If you go through the link completely different implementation route, you might need to create and link a different library which contains the faked implementation for several test cases. It also works well with pure header code.
- mehrdadn 8y agoIf I understand you correctly aren't you violating the initial assumption that there are only 2 implementations, one for production and one for testing? At which point, you're answering a different question...
- stinos 8y agoIt seems like a win all around. Depends. Virtual calls can be done purely in code, with different implementation files you need to go round via the build system. The former just always works independent of platform whereas often for the latter it's more configuration work. Also, ignoring YAGNI, you could say the virtual one makes it easy to quickly test things out etc, is easily mockable, ...
- krasin 8y ago>Out of curiosity, why use virtual calls in those cases? Why not just have two different implementation files -- one for tests, and one for production -- and choose one based on which one you're doing? It seems like a win all around. I don't fully understand the proposal here. Do you have an example of a code in the wild that uses the proposed scheme?
- mehrdadn 8y agoI'm just saying do conditional compilation instead of virtual dispatch, since you generally shouldn't really need both the test and production implementations to run inside the same program. So if you have reason to require // widget.h class Widget { virtual void throb(); }; // widget.cc class WidgetImpl : public Widget { void throb() { ... } }; class WidgetTest : public Widget { void throb() { ... } }; then, instead of that, just do // widget.h class Widget { void throb(); }; // widget.cc void Widget::throb() { ... } // widget.test.cc void Widget::throb() { ... } where you only compile widget.cc for the production build, and only compile widget.test.cc for the test build. Feel free to post a more specific example and I can take a stab at seeing if I can make this transformation to it (or why it might not be possible, if you think it's not).
- krasin 8y agoThank you for elaborating your proposal. It might work in a theoretical scenario, but unlikely to be practical for existing projects with millions lines of codes. Essentially, it was easier to write a compiler / linker optimization, than change the source code. On the bright side, everyone wins!
- mehrdadn 8y ago> It might work in a theoretical scenario, but unlikely to be practical for existing projects with millions lines of codes. I mean there's no reason you have to change everything by tomorrow. You could introduce it as a policy moving forward, the same way I'm sure you make any other changes to other patterns that need to be followed. I'm sure you have a process for this? > Essentially, it was easier to write a compiler / linker optimization, than change the source code. I agree, but see above and below. > On the bright side, everyone wins! Well, you waste space, time, and energy making the compiler and linker do work that doesn't inherently need to be done. You also lose parallelizability of the compilation of the two implementations if they currently reside in the same file. Finally the compiler (er, linker) might not be able to actually apply the devirtualizations you expect, whereas in this case it's just a matter of inlining since the target method is already known.
- yzmtf2008 8y agoWhat if I want both? What if in ClassATest, I need ClassBImpl? Do I go around creating more compile cases? Because that’s what we need, more magic in the build system.
- mehrdadn 8y agoNot sure I see what you're thinking of... could you give me a code example?
- pdpi 8y agoThe implementation of TestWidget only defines some stuff of its own but otherwise shares a bunch of code with Widget is a possible scenario here.
- mehrdadn 8y agoYou can still derive classes when you need to, right?
- pdpi 8y agoWhat class are you deriving from though? Can’t be Widget, because in the scenario you described Widget and TestWidget are mutually exclusive (TestWidget just replaces Widget altogether). The alternative is extracting the shared behaviour to a BaseWidget that both classes extend, but that’s just ugly as sin.
- mehrdadn 8y agoidk, would something like this work maybe? // widget.h class Widget { void throb1(); virtual void throb2(); }; // widget.cc void Widget::throb1() { ... } void Widget::throb2() { ... } // widget.test.cc void Widget::throb1() { ... } void Widget::throb2() { ... } class TestWidget : public Widget { int x; void throb2(); }; void TestWidget::throb2() { ... Widget::throb2(); ... } If not, could you present code to illustrate the problem? I won't try to address more issues without actual code.
- tzahola 8y agoOr just use a macro?
- hohenheim 8y agoYeah that would work but also make your code base a mess. Things that you can catch during build step and move in, or out, is much better handled at that level rather than leaking it into your code base.
- yoklov 8y agoOne reason is that other test might need the production version. Building twice for that case is more of a pain and not really worth it.
- mehrdadn 8y agoI mean, then you can avoid it in that situation. Or actually use the production version like I explained here: https://news.ycombinator.com/item?id=17505358 https://news.ycombinator.com/item?id=17505358 Or find another way to use it. It's not all-or-nothing.