Bug 279498 – REGRESSION (282580@main): 4.5MB binary size increase

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Antti Koivisto]

Apparently all the templates are generating way more code.

[Antti Koivisto]

rdar://135344420

[Antti Koivisto]

+4.5MB on WebCore
+4.5MB on WebCore_iosMac

[Sam Weinig]

Oof, that's quite a bit! Let me think if there is a good way to reduce this without a total re-do.

[Sam Weinig]

Took a little look at this, and it looks like the majority of the growth is in, of all places, the macho strings table.

Looking into this more, it seems there is a relatively well known issue with std::visit, at least as implemented by libc++.

A good resource, with links to other discussions and solutions is https://mpark.github.io/programming/2019/01/22/variant-visitation-v2/.

I'm going to try using the mpark::variant/visitor (https://github.com/mpark/variant) for the CSSCalc code and see what effect that has on code size. There may end up being a more surgical fix, but this is an interesting first step.

[Sam Weinig]

One additional link, https://github.com/llvm/llvm-project/issues/62648, is an issue tracked for libc++ on improving this.

[Sam Weinig]

Switching to mpark::variant reduced the total size change from:

  +4.2% +4.47Mi  +4.2% +4.47Mi    TOTAL

to

  +0.3%  +371Ki  +0.3%  +368Ki    TOTAL

Much more reasonable. (still a bit high, but not crazy).

Switch all of WebCore over to it would probably be a pretty substantial win.

[Gerald Squelart]

(Passer-by thought, please ignore if N/A)

Is the size regression mainly due to std::visit?
If yes, would it be possible to only change WTF::switchOn (which is used 6-7 times more often than naked std::visit) to accomplish most of the gains of mpark's re-implementation?

[Sam Weinig]

(In reply to Gerald Squelart from comment #7)
> (Passer-by thought, please ignore if N/A)
> 
> Is the size regression mainly due to std::visit?
> If yes, would it be possible to only change WTF::switchOn (which is used 6-7
> times more often than naked std::visit) to accomplish most of the gains of
> mpark's re-implementation?

Yeah, that was what I meant to imply with "a more surgical fix". I was hoping I could just call mpark::visit(), and pass it a std::variant, but that doesn't work, so will need to lift the implementation up and refactor it so it can work with std::variant, which is doable, but a bit tricky.

[Sam Weinig]

(I guess the unstated thing here is that switching to use mpark::variant for the calc code was a much easier test than refactoring WTF::switchOn, so that's why I did that as the test).

[Sam Weinig]

Pull request: https://github.com/WebKit/WebKit/pull/33706

[EWS]

Committed 283806@main (6aa472c27a8a): <https://commits.webkit.org/283806@main>

Reviewed commits have been landed. Closing PR #33706 and removing active labels.

[EWS]

Committed 283286.87@safari-7620-branch (cf65dbb69374): <https://commits.webkit.org/283286.87@safari-7620-branch>

Reviewed commits have been landed. Closing PR #1838 and removing active labels.