Bug 77510

Created attachment 124931 [details] WIP patch

Uploaded a WIP patch. The patch works fine in a clean build. However, when I touch Element.idl and try rebuild, makefile goes into infinite loop. This is strange since the dependency of makefile should be DAG. I've been debugging.

Created attachment 124938 [details] WIP patch Fixed the infinite loop issue by handling JavaScriptCallFrame.idl specially

Comment on attachment 124938 [details] WIP patch View in context: https://bugs.webkit.org/attachment.cgi?id=124938&action=review Looks like a good approach. The file name supplemental_dependency.tmp seems like it could be better. No reason the .tmp extension is particularly good. But I guess you already had that file name before this patch. > Source/WebCore/bindings/scripts/update-idl-needs-rebuild.pl:68 > + open FH, "> $file" or die "Couldn't open $file: $!"; Better syntax: open FH, ">", $file The version where the ">" is in the filename string is old/obsolete. Same for "<".

Adam, Darin: Thank you! Getting close to the goal, but one problem still remains. Even if I touch DOMWindowWebAudio.idl, DOMWindow.idl is not rebuilt. The reason is as follows. The build flow: > supplemental_dependency.tmp : $(ALL_IDLS) > perl resolve-supplemental.pl $(ALL_IDLS) > > %.idl-needs-rebuild : %.idl | supplemental_dependency.tmp > touch %.idl-needs-rebuild > perl update-idl-needs-rebuild.pl $< $@ # update-idl-needs-rebuild.pl touches the "main" IDL's .idl-needs-rebuild if the .idl-needs-rebuild is older than %.idl > > JS%.cpp : %.idl-needs-rebuild | *.idl-needs-rebuild > perl generate-bindings.pl supplemental_dependency.tmp $< # Generates JS%.cpp In particular, let us focus on this line: > JSDOMWindow.cpp : DOMWindow.idl-needs-rebuild | *.idl-needs-rebuild > ... At this point, the following five things can happen: (a) All *.idl-needs-rebuild rules fire. (b) The DOMWindow.idl-needs-rebuild rule fires. (c) The timestamps of JSDOMWindow.cpp and DOMWindow.idl-needs-rebuild are compared. (d) If JSDOMWindow.cpp is older than DOMWindow.idl-needs-rebuild, the JSDOMWindow.cpp rule is decided to fire. (e) If the JSDOMWindow.cpp rule is decided to fire, the JSDOMWindow.cpp rule fires. We were expecting that the following order would be guaranteed: (a)->(c), (b)->(c), (c)->(d), (d)->(e) However, actually what makefile guarantees is the following order: (a)->(e), (b)->(c), (c)->(d), (d)->(e) That way, if things happen in the order of (b)->(c)->(d)->(a)->(e), the JSDOMWindow.cpp rule does not fire. This actually happens in my experiments (even with -j 1). You can confirm the behavior in the following simplified example: all : A A : B | C echo A touch A B : echo B C : sleep 2 echo C touch B If you run the makefile with -j 1, "echo A" will be output in the odd runs but will not be output in the even runs. Any idea?

Adam, Darin: dominicc@ suggested the following idea: supplemental_dependency.tmp : $(ALL_IDLS) perl resolve-supplemental.pl ... %.idl-needs-rebuild : %.idl supplemental_dependency.tmp perl update-idl-needs-rebuild.pl ... JS%.cpp : %.idl-needs-rebuild perl generate-bindings.pl ... resolve-supplemental.pl generates the following supplemental_dependency.tmp. The number next to a file name is the timestamp of the file. A.idl(1200) B.idl(1000) B-supplemental1.idl(800) B-supplemental2.idl(1200) C.idl(1000) update-idl-needs-rebuild.pl touches %.idl-needs-rebuild so that the timestamps of %.idl-needs-rebuild and %.idl become equal, if (1) %.idl-needs-rebuild is older than %.idl or (2) there is an IDL file (other than %.idl) whose timestamp is newer than %.idl in the %.idl's line of supplemental_dependency.tmp. For example, if the timestamps of A.idl-needs-rebuild, B.idl-needs-rebuild and C.idl-needs-rebuild are 1000, then A.idl-needs-rebuild and B.idl-needs-rebuild will be touched. If any IDL is updated, resolve-supplemental.pl will run once, update-idl-needs-rebuild.pl will run for all IDL files, and generate-bindings.pl will run for the IDL files which are modified or whose dependencies are changed. It appears it will work fine. The advantage of this idea is that it does not use makefile's tricky syntax. Thoughts? If you think this will work fine, I am happy to implement it.

The only immediate problem I spot is that running the update-idl-needs-rebuild.pl script hundreds of times might be slow. Otherwise, sounds like it should be OK.

Created attachment 125607 [details] Patch

(In reply to comment #7) > The only immediate problem I spot is that running the update-idl-needs-rebuild.pl script hundreds of times might be slow. Darin, Adam: I confirmed that the uploaded patch works as we expect, except that update-idl-needs-rebuild.pl always runs for all IDL files even if no IDL files are updated. (If no IDLs are updated, update-idl-needs-rebuild.pl just runs and does nothing. It takes about 3 seconds in my local AppleWebKit. Would it be acceptable?)

Comment on attachment 125607 [details] Patch IMHO, this is an improvement. It's probably not the final solution. We can continue to iterate in another patch.

Comment on attachment 125607 [details] Patch Clearing flags on attachment: 125607 Committed r106862: <http://trac.webkit.org/changeset/106862>

All reviewed patches have been landed. Closing bug.

Reverted r106862 for reason: Mac build fails if we manually remove generated code Committed r106913: <http://trac.webkit.org/changeset/106913>

Sadly I rolled out the patch because the patch will break AppleWebKit build. > supplemental_dependency.tmp : $(ALL_IDLS) > perl resolve-supplemental.pl ... > > %.idl-needs-rebuild : %.idl supplemental_dependency.tmp > perl update-idl-needs-rebuild.pl ... > > JS%.cpp : %.idl-needs-rebuild > perl generate-bindings.pl ... In the above build flow, if we manually remove JS%.cpp, the JS%.cpp will be never rebuilt again. (The same happens for JS%.h, WebDOM%.h, WebDOM%.cpp, etc) I tried several approaches to fix the issue, but could not find a sane approach.

I think this problem is gone.