1369420 - Identical failure paths should be collapsed

Status: RESOLVED WORKSFORME

(Core :: CSS Parsing and Computation, enhancement, P3)

Description

[(Away)]

In the disassembly of style::properties::PropertyDeclaration::parse_into, at the end there is an area with hundreds of repetitions of:

142476 00000001`80437946 e8b5490f00      call    xul!alloc::oom::oom (00000001`8052c300)
142476 00000001`8043794b 0f0b            ud2
142476 00000001`8043794d e8ae490f00      call    xul!alloc::oom::oom (00000001`8052c300)
142476 00000001`80437952 0f0b            ud2
142476 00000001`80437954 e8a7490f00      call    xul!alloc::oom::oom (00000001`8052c300)
142476 00000001`80437959 0f0b            ud2
...

I'm guessing what's happening is that the compiler places unlikely fatal codepaths at the end of the function, outside of the normal control flow (which is good) but it would be good if it could recognize when several of these paths will lead to the same place and collapse them to save space.

(Such an optimization would be incorrect if we're using the return address as a differentiator for which particular OOM occurred, but I don't believe that's the case here.)

This was on a Win64 opt build, if it matters.

Comment 1

[Xidorn Quan [:xidorn] UTC+10]

This sounds like something Rust or LLVM should optimize? It doesn't seem to me this is something optimizable in our code.

Comment 2

[Bobby Holley (:bholley)]

Marking this a tooling bug per comment 1, NI dmajor to verify.

Comment 3

[(Away)]

Yep.

Comment 4

[Xidorn Quan [:xidorn] UTC+10]

Should we ask some Rust guys to take a look at this problem?

Comment 5

[Bobby Holley (:bholley)]

How much would it win us?

Comment 6

[(Away)]

Hard to say; it varies based on function.

style::properties::PropertyDeclaration::parse_into is about 1% repeated-oom-gunk (5KB).
style::properties::{{impl}}::clone has about 6 pages of oom-gunk compared to 3 pages of actual code!
Some other random functions I dumped don't have any repetition at all.

Wild guess, maybe that 1% number is reasonable to extrapolate from?

Comment 7

[Bobby Holley (:bholley)]

That seems significant enough to pursue. Brian, can you route this and bring it up with the right people?

Comment 8

[Brian Anderson]

I have cross-posted a bug and pinged somebody who might be interested: https://github.com/rust-lang/rust/issues/42666

Comment 9

[Firefox Bug Husbandry Bot]

https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Move_fix-optionals

Comment 10

[Mike Hommey [:glandium]]

This doesn't seem to be happening anymore.