279498 2024-09-11 01:13:16 -0700 REGRESSION (282580@main): 4.5MB binary size increase 2024-09-18 18:22:54 -0700 1 1 1 Unclassified WebKit CSS WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=278244 InRadar P2 Normal --- 1 koivisto sam cdumez darin g_squelart koivisto sam simon.fraser webkit-bug-importer oldest_to_newest 2059231 0 koivisto 2024-09-11 01:13:16 -0700 Apparently all the templates are generating way more code. 2059232 1 koivisto 2024-09-11 01:13:33 -0700 rdar://135344420 2059233 2 koivisto 2024-09-11 01:19:38 -0700 +4.5MB on WebCore +4.5MB on WebCore_iosMac 2059307 3 sam 2024-09-11 09:09:57 -0700 Oof, that's quite a bit! Let me think if there is a good way to reduce this without a total re-do. 2059465 4 sam 2024-09-11 16:49:26 -0700 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. 2059479 5 sam 2024-09-11 17:19:07 -0700 One additional link, https://github.com/llvm/llvm-project/issues/62648, is an issue tracked for libc++ on improving this. 2059488 6 sam 2024-09-11 17:28:14 -0700 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. 2059512 7 g_squelart 2024-09-11 18:11:01 -0700 (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? 2059678 8 sam 2024-09-12 08:00:55 -0700 (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. 2059679 9 sam 2024-09-12 08:02:27 -0700 (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). 2060479 10 sam 2024-09-16 10:48:19 -0700 Pull request: https://github.com/WebKit/WebKit/pull/33706 2060879 11 ews-feeder 2024-09-17 16:14:29 -0700 Committed 283806@main (6aa472c27a8a): <https://commits.webkit.org/283806@main> Reviewed commits have been landed. Closing PR #33706 and removing active labels. 2061292 12 ews-feeder 2024-09-18 18:22:54 -0700 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.