1646937 - The webidl generation script takes 10s

Status: NEW

(Firefox Build System :: General, task, P3)

Description

[Mike Hommey [:glandium]]

It would be good to look whether we can split it (it currently handles all the files at once), or if it could be made faster some other way.

Comment 1

[Nathan Froyd [:froydnj]]

ISTR that there are inherent parts of codegen that require a global view of all web-related APIs (e.g. you can define partial interfaces on Window). I don't know whether we could run just the part that required global views and then parallelize the actual codegen (similar to the way ThinLTO works), or whether the part that requires a global view(s) is really the limiting factor.

CC'ing people who might be able to offer perspective here.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Another option may be to just use something faster than python for the webidl codegen. Of course that may not be trivial... Though it seems there are some webidl parsers available like weedle.

I guess such a thing would mean that we need to download the "codegen tool" during bootstrap or something, and changes to the code generator may become a bit harder (as in, require changes to your mozconfig to point to your modified binary).

Not sure how often the WebIDL code generation changes, I guess that'd be one thing to consider. Feature parity may or may not be easy to achieve either...

Josh, IIRC at some point Servo folks were considering moving away from WebIDL.py into some rust-only alternative... Is that right? Or am I misremembering? What were the roadblocks for that?

Comment 3

[Josh Matthews [:jdm]]

No serious thought was put into it besides "wouldn't it be nicer if the codegen was in rust, rather than python?" The main roadblock was rewriting the existing 10k lines of python, especially as we would then lose the ability to compare our codegen script against Gecko to help track down bugs.

Comment 4

[Ricky Stewart]

Throwing this into the build system triage just at least to investigate the cause of the slowness, do some profiling, and potentially propose low-hanging fruit fixes.

Comment 5

[Ricky Stewart]

Attachment: webidl.prof

Profiling results attached.

Nothing immediately jumps out to me as being obviously, completely wrong here. Somebody with a better understanding of webidl.py and its inner workings may have a different perspective (e.g. they might see that something is a bottleneck when it clearly is not expected to be), so if that's you I would encourage you to take a look.

General Python slowness is obviously an issue here. Like a lot of Python scripts that get run during the build, it primarily does string manipulation, which Python is notoriously slow at. So in the absence of architectural changes, there's definitely a floor for performance here. Having said that, here are a couple things that immediately jump out to me:

1. We spend quite a lot of time in ply (say, 15% of the total runtime). I assume this could be optimized, but it is third-party. Looks like we're parsing about 1500 files here -- that's not a trivial amount, but overall for this build about 25% of the total runtime was spent in parsing (inside of ply in addition to all the other stuff in _parse_webidl(). That doesn't seem right to me.

2. Presumably parallelizing into many different build jobs for generating each of the result .cpp files is possible. We would need one job that parses everything and then pickles the result data into a file in the objdir that the N build jobs could then consume so they don't each have to repeat the expensive parsing step. Of course, a) this wouldn't give any additional incrementality -- any change to any of the .webidl files would require everything to be rebuilt, and b) the parsing and pickling is still a bottleneck. This is presumably a big improvement in terms of E2E latency but isn't a silver bullet.

3. We spend a lot of time in string manipulation and unfortunately Python is just uniformly awful at this task. This is exacerbated by the fact that we produce the file contents as a string first before writing the contents to a file, which is an anti-pattern IMO. Instead the dependency should be injected here so the file contents are written out incrementally to the destination file rather than producing a bunch of garbage with a string of expensive incremental string joins.

4. There's a bespoke enum implementation for which the accessors take a decent amount of time, primarily because it's all dynamic -- this is a well-known non-performant pattern in Python metaprogramming. The way to do it is to remove runtime dynamism to whatever extent possible. Presumably the enum stdlib package is more performant so this should probably be a drop-in replacement.