Bug 40350 – Attempt to speed up builds by reducing header dependencies

• Status: RESOLVED
• Resolution: WONTFIX
• Component: New Bugs

[Eric Seidel (no email)]

Attempt to speed up builds by reducing header dependencies

[Eric Seidel (no email)]

Created attachment 58211 [details]
Patch

[Adam Barth]

Comment on attachment 58211 [details]
Patch

Ok.  Does this actually make the build faster?

[Eric Seidel (no email)]

I believe so.  Difficult to test since full builds take 1hr+ on my laptop atm.

[Eric Seidel (no email)]

Created attachment 58213 [details]
Patch for landing

[WebKit Review Bot]

Attachment 58211 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/3192058

[Eric Seidel (no email)]

Comment on attachment 58213 [details]
Patch for landing

I'm going to need the EWS results.  Marking back to r?

[WebKit Review Bot]

Attachment 58213 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/3226083

[David Levin]

Comment on attachment 58213 [details]
Patch for landing

r- due to ews build break.

[Eric Seidel (no email)]

Thanks. :)  I'll figure out what header is depending on PassRefPtr and missing the include and upload a new patch.

[Eric Seidel (no email)]

Created attachment 58305 [details]
Should now compile for Chromium

[Eric Seidel (no email)]

Comment on attachment 58305 [details]
Should now compile for Chromium

Waiting for the EWS bots to roll green then this can be r+'d and cq+'d.

[WebKit Review Bot]

Attachment 58305 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/3239091

[David Kilzer (:ddkilzer)]

(In reply to comment #11)
> (From update of attachment 58305 [details])
> Waiting for the EWS bots to roll green then this can be r+'d and cq+'d.

How did you test for performance regressions?

I think the "outlined" method changes should be in a separate patch.

[David Levin]

Comment on attachment 58305 [details]
Should now compile for Chromium

r- due to ews build break.

Plus Dave had a good point about the methods that have been uninlined and possible perf impact -- would be nice to do that in a separate patch.

[Balazs Kelemen]

Personally, what is annoying me about this patch is the comments about includes. Do we really need those? Generally, we have got scarce comments in the code base. I have not seen explanations about why we include something somewhere yet. Actually, I would be very happy to see more comments in webkit to make the code more readable, but I think this kind of comments gives little useful information to the reader.

[Eric Seidel (no email)]

This is a long-abandoned patch and ma no longer be useful.

[Ahmad Saleem]

@Darin - did a lot of similar activities in RenderStyle and now WebPage. So I think it might to be needed anymore.

Can we close this? Unless @Darin, you think we can use this or something similar from it.

[Darin Adler]

It’s too old to be useful, yes. We should do this sort of thing, but this patch isn’t where we will start.