Avoid codesize impact of the pushing in PropertyDeclaration::parse

[Manishearth]

While investigating https://bugzilla.mozilla.org/show_bug.cgi?id=1297322#c19, I realized that the asm generated for this function in release mode is abominable. LLVM, always wanting to please, inlines a bajillion things resulting in 100k lines of ASM with a lot of redundant bits. We have a thousand calls to alloc::oom at the end of the function alone. I'm told that LLVM doesn't hoist things out of switches that well, which might be the case here. The only common allocation here is the pushing (parsing may allocate but that's not common).

I thought I'd hoist the allocation calls out. All the longhands can share a single push() call.

Furthermore, the shorthands have a bunch of sequential push calls for CSS-wide keywords. I'm not sure how well LLVM optimizes those, but we should be reserve()ing early anyway; pretty sure reserve(n) followed by n push()es when inlined will make the push a trivial pointer-bump.

There's a further optimization which I have yet to implement that @dotdash pointed out that will let me hoist the push calls for all shorthands into one (not counting any pushes done by shorthand parsing). This should have no perf impact but reduce code size further.

I haven't yet measured the impact here (@mbrubeck if you have time would you be able to check how this impacts XUL?), but will do so later today.

r? @mbrubeck

---

This change is 

[highfive]

Heads up! This PR modifies the following files:

• @bholley: components/style/properties/properties.mako.rs
• @emilio: components/style/properties/properties.mako.rs

[highfive]

Warning

• These commits modify style code, but no tests are modified. Please consider adding a test!

[SimonSapin]

Looks good to me.

@bors-servo r+

[bors-servo]

📌 Commit 77e132b has been approved by SimonSapin

[bors-servo]

⌛ Testing commit 77e132b with merge a5128a5...

[Manishearth]

@bors-servo r-

not yet 😄

[mbrubeck]

In a build of the incubator/stylo repo at changeset 67109dc09a80, the first patch here decreases the size of PropertyDeclaration::parse by about 1 KiB, but increases total libxul text segment size by 320 bytes (compared to the same changeset without no patches applied).

The two patches together increase both PropertyDeclaration::parse and total libxul text segment size by about 6 KiB.

[Manishearth]

@dotdash any idea why the above would happen? (libxul is the binary this gets compiled into. debug is off)

[bors-servo]

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-gnu-dev, windows-msvc-dev

[Ms2ger]

This certainly should have a comment about the reason for the somewhat curious code flow

[Manishearth]

It will, if it can be made to actually work 😄

[jdm]

What's the plan for this PR?

[Manishearth]

It didn't improve anything, and the .reserve() only has a rare perf benefit (nobody uses CSS keywords in shorthands, and if they do a couple of reallocs is nbd), so closing.

[mbrubeck]

@Manishearth, are there (or should there be) any bugs filed on rustc or LLVM for things they could do better here?

[Manishearth]

.reserve() plus a bunch of .push()es should be inlined+optimized into a single bounds check/realloc and a bunch of memmoves. It seems like currently the realloc/bounds check code isn't removed in case of the pushes. No idea how hard this is to fix.

@dotdash @eddyb thoughts?

[SimonSapin]

If the number of push calls is known statically (i.e. there’s no loop), could this whole pattern be replaced with one extend_from_slice call?

[Manishearth]

Moved to #15279

(can't reuse PR after the branch has been deleted)