Bug 231473 - [Build-time perf] Speed up the WebCore null build
Summary: [Build-time perf] Speed up the WebCore null build
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-10-09 01:02 PDT by Jer Noble
Modified: 2021-10-12 08:48 PDT (History)
8 users (show)

See Also:


Attachments
Patch (29.87 KB, patch)
2021-10-09 11:58 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (29.86 KB, patch)
2021-10-09 12:00 PDT, Jer Noble
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2021-10-09 01:02:33 PDT
[Build-time perf] Speed up the WebCore null build
Comment 1 Jer Noble 2021-10-09 11:58:45 PDT
Created attachment 440715 [details]
Patch
Comment 2 Jer Noble 2021-10-09 12:00:51 PDT
Created attachment 440716 [details]
Patch
Comment 3 Myles C. Maxfield 2021-10-09 23:53:29 PDT
How can we make sure this doesn’t happen again? People delete files all the time.
Comment 4 EWS 2021-10-10 00:01:44 PDT
Committed r283873 (242750@main): <https://commits.webkit.org/242750@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 440716 [details].
Comment 5 Radar WebKit Bug Importer 2021-10-10 00:02:17 PDT
<rdar://problem/84069473>
Comment 6 mitz 2021-10-10 08:15:44 PDT
Comment on attachment 440716 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=440716&action=review

> Source/WebCore/ChangeLog:14
> +        The "Make Frameworks Symbolic Link" build phase had no output defined, causing it to run every time.

Isn’t it still going to run every time when building for iOS and in other configurations where WebCore.framework/Frameworks doesn’t exist and doesn’t get created?
Comment 7 Tim Horton 2021-10-10 13:25:52 PDT
Will FRAMEWORKS_FOLDER_PATH do the trick?
Comment 8 Darin Adler 2021-10-11 13:17:23 PDT
Comment on attachment 440716 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=440716&action=review

> Source/WebCore/ChangeLog:12
> +        The "Check For Inappropriate Objective-C Class Names" and "Check For Inappropriate Files In Framework" build phases
> +        had no inputs or outputs listed, causing them to run every time. Define an output of an empty text file in the
> +        TARGET_TEMP_DIR directory that will get touched after the check, allowing the build system to do dependency checking.

Given no inputs were listed, what dependencies are being checked? I don’t understand the concept of checking dependencies without inputs.
Comment 9 mitz 2021-10-11 13:23:24 PDT
(In reply to Darin Adler from comment #8)
> Comment on attachment 440716 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=440716&action=review
> 
> > Source/WebCore/ChangeLog:12
> > +        The "Check For Inappropriate Objective-C Class Names" and "Check For Inappropriate Files In Framework" build phases
> > +        had no inputs or outputs listed, causing them to run every time. Define an output of an empty text file in the
> > +        TARGET_TEMP_DIR directory that will get touched after the check, allowing the build system to do dependency checking.
> 
> Given no inputs were listed, what dependencies are being checked? I don’t
> understand the concept of checking dependencies without inputs.

As far as I can tell, those build phases both list $(TARGET_BUILD_DIR)/$(EXECUTABLE_PATH) as their input.
Comment 10 Darin Adler 2021-10-11 13:27:42 PDT
Comment on attachment 440716 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=440716&action=review

>>> Source/WebCore/ChangeLog:12
>>> +        TARGET_TEMP_DIR directory that will get touched after the check, allowing the build system to do dependency checking.
>> 
>> Given no inputs were listed, what dependencies are being checked? I don’t understand the concept of checking dependencies without inputs.
> 
> As far as I can tell, those build phases both list $(TARGET_BUILD_DIR)/$(EXECUTABLE_PATH) as their input.

Good! Jer’s comment said there were no inputs listed and I didn’t fact check.
Comment 11 Jer Noble 2021-10-12 08:30:47 PDT
Apologies to everyone; I didn't see your comments till just now.

(In reply to mitz from comment #6)
> Comment on attachment 440716 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=440716&action=review
> 
> > Source/WebCore/ChangeLog:14
> > +        The "Make Frameworks Symbolic Link" build phase had no output defined, causing it to run every time.
> 
> Isn’t it still going to run every time when building for iOS and in other
> configurations where WebCore.framework/Frameworks doesn’t exist and doesn’t
> get created?

Yes. I didn't completely solve this problem. There doesn't seem to be a mechanism to mark build phases as platform dependent. Given how Xcode's dependency checking works, this phase would need a (e.g.) "WEBCORE_FRAMEWORK_PATH" defined as `$(TARGET_BUILD_DIR)/WebCore.framework/Frameworks` on macOS and some other path (like, `$(TARGET_TEMP_DIR)/DoesntNeedASymlink`) on other platforms.

(In reply to Darin Adler from comment #8)
> Comment on attachment 440716 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=440716&action=review
> 
> > Source/WebCore/ChangeLog:12
> > +        The "Check For Inappropriate Objective-C Class Names" and "Check For Inappropriate Files In Framework" build phases
> > +        had no inputs or outputs listed, causing them to run every time. Define an output of an empty text file in the
> > +        TARGET_TEMP_DIR directory that will get touched after the check, allowing the build system to do dependency checking.
> 
> Given no inputs were listed, what dependencies are being checked? I don’t
> understand the concept of checking dependencies without inputs.

Apologies, this comment was incorrect, as Mitz pointed out.
Comment 12 Jer Noble 2021-10-12 08:48:01 PDT Comment hidden (obsolete)
Comment 13 Jer Noble 2021-10-12 08:48:01 PDT
(In reply to Myles C. Maxfield from comment #3)
> How can we make sure this doesn’t happen again? People delete files all the
> time.

There's a script that runs as part of the build that adds new inputs and outputs to the DerivedSources-inputs/outputs.xcfilelist. That script would need to be modified to check for deletions/renames rather that just additions.