Bug 231473

Summary: [Build-time perf] Speed up the WebCore null build
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: New BugsAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, eric.carlson, jean-yves.avenard, mitz, mmaxfield, peng.liu6, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Jer Noble
Reported 2021-10-09 01:02:33 PDT
[Build-time perf] Speed up the WebCore null build
Attachments
Patch (29.87 KB, patch)
2021-10-09 11:58 PDT, Jer Noble
no flags
Patch (29.86 KB, patch)
2021-10-09 12:00 PDT, Jer Noble
no flags
Jer Noble
Comment 1 2021-10-09 11:58:45 PDT
Jer Noble
Comment 2 2021-10-09 12:00:51 PDT
Myles C. Maxfield
Comment 3 2021-10-09 23:53:29 PDT
How can we make sure this doesn’t happen again? People delete files all the time.
EWS
Comment 4 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].
Radar WebKit Bug Importer
Comment 5 2021-10-10 00:02:17 PDT
mitz
Comment 6 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?
Tim Horton
Comment 7 2021-10-10 13:25:52 PDT
Will FRAMEWORKS_FOLDER_PATH do the trick?
Darin Adler
Comment 8 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.
mitz
Comment 9 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.
Darin Adler
Comment 10 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.
Jer Noble
Comment 11 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.
Jer Noble
Comment 12 2021-10-12 08:48:01 PDT Comment hidden (obsolete)
Jer Noble
Comment 13 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.
Note You need to log in before you can comment on or make changes to this bug.