Bug 206720

Summary: Break the dependency between jsc and DerivedSources
Product: WebKit Reporter: Robin Morisset <rmorisset>
Component: JavaScriptCoreAssignee: Robin Morisset <rmorisset>
Status: REOPENED ---    
Severity: Normal CC: ap, commit-queue, ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 206566, 206758, 206851    
Bug Blocks:    
Attachments:
Description Flags
Patch
mark.lam: review+, mark.lam: commit-queue-
Patch none

Description Robin Morisset 2020-01-23 17:18:17 PST 
According to Tadeu, the slowdown of production builds (by more than 20%) when he landed his bytecode patch was caused by a new dependency of the jsc binary on DerivedSources/, which was needed because of DerivedSources/BytecodeStructs.h being included in CommonSlowPaths.h which is transitively included in jsc.cpp.
Now that BytecodeStructs.h is no longer included in CommonSlowPaths.cpp (see https://bugs.webkit.org/show_bug.cgi?id=206566), I'd like to see if the dependency can be safely removed, to recover this compile time regression.
Comment 1 Robin Morisset 2020-01-23 17:27:46 PST 
Created attachment 388627 [details]
Patch
Comment 2 Mark Lam 2020-01-23 20:03:59 PST 
Comment on attachment 388627 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:9
> +        Now that BytecodeStructs.h is no longer included in CommonSlowPaths.cpp (see https://bugs.webkit.org/show_bug.cgi?id=206566), I'm trying to break the dependency, to recover from this compile time regression.

I think you mean CommonSlowPaths.h here.
Comment 3 Robin Morisset 2020-01-23 20:06:30 PST 
(In reply to Mark Lam from comment #2)
> Comment on attachment 388627 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=388627&action=review
> 
> > Source/JavaScriptCore/ChangeLog:9
> > +        Now that BytecodeStructs.h is no longer included in CommonSlowPaths.cpp (see https://bugs.webkit.org/show_bug.cgi?id=206566), I'm trying to break the dependency, to recover from this compile time regression.
> 
> I think you mean CommonSlowPaths.h here.

Yes, it was a typo
Comment 4 Robin Morisset 2020-01-23 20:08:25 PST 
Created attachment 388646 [details]
Patch

Fixed the typo in the changelog.
Comment 5 WebKit Commit Bot 2020-01-23 20:52:34 PST 
Comment on attachment 388646 [details]
Patch

Clearing flags on attachment: 388646

Committed r255052: <https://trac.webkit.org/changeset/255052>
Comment 6 WebKit Commit Bot 2020-01-23 20:52:36 PST 
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2020-01-23 20:53:13 PST 
<rdar://problem/58858767>
Comment 8 WebKit Commit Bot 2020-01-24 09:55:10 PST 
Re-opened since this is blocked by bug 206758