Closed
Bug 704788
Opened 13 years ago
Closed 13 years ago
Help compiler ignore mDebugMode branches on non-debug builds
Categories
(Core :: Graphics: CanvasWebGL, defect)
Core
Graphics: CanvasWebGL
Tracking
()
RESOLVED
FIXED
mozilla11
People
(Reporter: jgilbert, Assigned: jgilbert)
Details
Attachments
(1 file)
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.
Assignee | ||
Comment 1•13 years ago
|
||
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)
Assignee | ||
Comment 2•13 years ago
|
||
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?
Comment 3•13 years ago
|
||
(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 4•13 years ago
|
||
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?
Updated•13 years ago
|
Attachment #576442 -
Flags: review?(bjacob) → review+
Assignee | ||
Comment 5•13 years ago
|
||
I would feel better about being gutsy if we fix bug 697810, where we can be even more sure it's being inlined.
Assignee | ||
Comment 6•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/84f4cb3c8b9e
Assignee: nobody → jgilbert
Target Milestone: --- → mozilla11
Comment 7•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/84f4cb3c8b9e
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•