Closed Bug 704788 Opened 8 years ago Closed 8 years ago

Help compiler ignore mDebugMode branches on non-debug builds

Categories

(Core :: Canvas: WebGL, defect)

defect
Not set

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.
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.
https://hg.mozilla.org/integration/mozilla-inbound/rev/84f4cb3c8b9e
Assignee: nobody → jgilbert
Target Milestone: --- → mozilla11
https://hg.mozilla.org/mozilla-central/rev/84f4cb3c8b9e
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.