Bug 231473 – [Build-time perf] Speed up the WebCore null build

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

[Jer Noble]

[Build-time perf] Speed up the WebCore null build

[Jer Noble]

Created attachment 440715 [details]
Patch

[Jer Noble]

Created attachment 440716 [details]
Patch

[Myles C. Maxfield]

How can we make sure this doesn’t happen again? People delete files all the time.

[EWS]

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].

[Radar WebKit Bug Importer]

<rdar://problem/84069473>

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?

[Tim Horton]

Will FRAMEWORKS_FOLDER_PATH do the trick?

[Darin Adler]

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.

(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.

[Darin Adler]

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.

[Jer Noble]

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.

[Jer Noble]

(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.

[Jer Noble]

(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.