1370209 - Allow --disable-rust-debug if --enable-debug is set

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Andrew Osmond [:aosmond] (he/him)]

--enable-debug implies --enable-rust-debug, which has a significant impact on performance for Rust code. While this is often acceptable, there are also times when it is very annoying. Specifically when using it with WebRender, the rendering performance can be abysmal under certain loads. On Linux it is possible with aggressive scrolling to get the browser into a locked up state because the rendering always behind vsync. Obviously we would want to avoid that, but for developers who only care about C++ assertions for their local builds, we shouldn't make them suffer in the meantime and in case of similar future scenarios :).

I propose adding --enable-cpp-debug which sets MOZ_DEBUG like --enable-debug does now, and making it implied by --enable-debug (so the default remains the same as it does now).

Comment 1

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Add --enable-cpp-debug to build (only) C++ code with assertions., v1

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ad23d3d71ce7035a2480d683832458726e3f0cd

Comment 2

[Mike Hommey [:glandium]]

Why not make --enable-debug --disable-rust-debug work, instead?

Comment 3

[Mike Hommey [:glandium]]

Also, --enable-cpp-debug implies it'd be limited to C++, where that's not actually the case. There are plenty of non-C++ stuff that use MOZ_DEBUG.

Comment 4

[Nathan Froyd [:froydnj]]

Comment on attachment 8874417 [details] [diff] [review]
Add --enable-cpp-debug to build (only) C++ code with assertions., v1

Review of attachment 8874417 [details] [diff] [review]:
-----------------------------------------------------------------

I agree that this would be useful, but it sounds like a little refinement is needed, given glandium's comments.

Comment 5

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Allow implied defaults to be overridden, v1

glandium, do you mean something like this?

Comment 6

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 8875400 [details] [diff] [review]
Allow implied defaults to be overridden, v1

Now I see this is dumb, I think I figured out what you meant :). Next patch forthcoming.

Comment 7

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Allow setting --disable-rust-debug when --enable-debug is used., v3

Comment 8

[Andrew Osmond [:aosmond] (he/him)]

Attachment: 0001-Bug-1370209-Allow-setting-disable-rust-debug-when-en.patch

Ugh turn off C++ brain, no explicit return None required.

Comment 9

[Nathan Froyd [:froydnj]]

Comment on attachment 8876278 [details] [diff] [review]
0001-Bug-1370209-Allow-setting-disable-rust-debug-when-en.patch

Review of attachment 8876278 [details] [diff] [review]:
-----------------------------------------------------------------

I think the patch makes sense, but have you tested it with all the combinations of --enable/disable-rust-debug and --enable/disable-debug (and maybe --enable/disable-optimize) to ensure that Rust code gets compiled the way you want?

r=me, assuming we have appropriate testing done per above.

Comment 10

[Andrew Osmond [:aosmond] (he/him)]

(In reply to Nathan Froyd [:froydnj] (out until 24 July) from comment #9)
> Comment on attachment 8876278 [details] [diff] [review]
> 0001-Bug-1370209-Allow-setting-disable-rust-debug-when-en.patch
> 
> Review of attachment 8876278 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I think the patch makes sense, but have you tested it with all the
> combinations of --enable/disable-rust-debug and --enable/disable-debug (and
> maybe --enable/disable-optimize) to ensure that Rust code gets compiled the
> way you want?
> 
> r=me, assuming we have appropriate testing done per above.

Today I got around to doing this. Built with all the combinations of the above flags, produced the appropriate MOZ_RUST_DEBUG flags and release vs debug rust crates.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9f61b0a0397e
Allow setting --disable-rust-debug when --enable-debug is used. r=froydnj

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9f61b0a0397e