Closed Bug 704788 Opened 8 years ago Closed 8 years ago
Help compiler ignore m
Debug Mode branches on non-debug builds
From bjacob: I would like the compiler to be able to take out this branch completely in release builds where debug mode isn't available at all. Proposal: introduce an *inline-defined* IsDebugMode() method, whose body has a #ifndef DEBUG return false #else... and edit code all over the place to use IsDebugMode() instead of using mDebugMode directly. This would also be nicer to hack: to force-enable debug mode, one would just have to edit that function, which would be nicely discoverable since it would be called everywhere debug mode is implemented.
https://tbpl.mozilla.org/?tree=Try&rev=3e95bc3ba163 I made the function just return the value of mDebugMode (instead of true/false) so that it can be used as a drop-in replacement.
Attachment #576442 - Flags: review?(bjacob)
I guess it doesn't need an 'inline' tag, since I now understand that it's implicitly inlined because it's defined in the class block. Should I remove the tag?
(In reply to Jeff Gilbert [:jgilbert] from comment #2) > I guess it doesn't need an 'inline' tag, since I now understand that it's > implicitly inlined because it's defined in the class block. Should I remove > the tag? It should be implicit indeed; also, for such a small function, the compiler should be good at inlining anyways. Up to you.
Comment on attachment 576442 [details] [diff] [review] Use inlined function to allow compile-time pruning of mDebugMode branches in release builds Review of attachment 576442 [details] [diff] [review]: ----------------------------------------------------------------- I was wondering if we'd be gutsy enough to remove the #ifdef DEBUG around the definition of BEFORE_GL_CALL and AFTER_GL_CALL? I.e. rely on the fact that, with your patch, the compiler can know that BeforeGLCall is a NOP when DEBUG is not defined. I'm pretty confident that this would work fine. What do you think?
Attachment #576442 - Flags: review?(bjacob) → review+
I would feel better about being gutsy if we fix bug 697810, where we can be even more sure it's being inlined.
Assignee: nobody → jgilbert
Target Milestone: --- → mozilla11
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.