Help compiler ignore mDebugMode branches on non-debug builds

RESOLVED FIXED in mozilla11

Status

()

Core
Canvas: WebGL
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jgilbert, Assigned: jgilbert)

Tracking

unspecified
mozilla11
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
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

6 years ago
Created attachment 576442 [details] [diff] [review]
Use inlined function to allow compile-time pruning of mDebugMode branches in release builds

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

6 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?
(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+
(Assignee)

Comment 5

6 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

6 years ago
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
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.