Bug 76970 – [meta] When any IDL is updated, all IDLs are rebuilt

• Component: WebCore JavaScript
• Status: RESOLVED CONFIGURATION CHANGED

[Kentaro Hara]

I've been trying to stop rebuilding .h/.cpp files generated by unchanged IDLs (bug 76836), but the approach was wrong. We should invalidate patches committed in r105697, r105766, r105809 and r105805.

In r105697, r105766, r105809 and r105805, I modified CodeGenerator*.pm so that they overwrite .h/.cpp files only when the bytes differ. By this fix, we were able to stop rebuilding .h/.cpp files that are not changed.

However, the fix has made generate-bindings.pl run for almost all IDLs every time. The reason is as follows:

(0) Assume that there are A.idl, B.idl and C.idl.

(1) Modify A.idl.
(2) First build.
(3) supplemental_dependency.tmp is updated.
(4) generate-bindings.pl runs for A.idl, B.idl and C.idl.
(5) A.h and A.cpp are updated. B.h, B.cpp, C.h and C.cpp are not updated.

(6) Second build.
(7) Since B.h, B.cpp, C.h and C.cpp are older than supplemental_dependency.tmp, generate-bindings.pl runs for B.idl and C.idl.
(8) B.h, B.cpp, C.h and C.cpp are not updated.

(9) Third build.
(10) Since B.h, B.cpp, C.h and C.cpp are older than supplemental_dependency.tmp, generate-bindings.pl runs for B.idl and C.idl.
(11) B.h, B.cpp, C.h and C.cpp are not updated.

...

We should fix the bug somehow, but how to fix it is not obvious to me. For the time being, let me commit a patch that invalidates r105697, r105766, r105809 and r105805.

[Darin Adler]

This is what I was driving at when I asked how we would write the makefiles! I don’t remember what bug that comment was in.

[Kentaro Hara]

Created attachment 123863 [details]
Patch

[Kentaro Hara]

(In reply to comment #1)
> This is what I was driving at when I asked how we would write the makefiles! I don’t remember what bug that comment was in.

Sorry, I noticed, now...

[Adam Barth]

Sounds like we need another plan to solve the original problem.

[Darin Adler]

Found it, <https://bugs.webkit.org/show_bug.cgi?id=74900#c20>.

[WebKit Review Bot]

Comment on attachment 123863 [details]
Patch

Clearing flags on attachment: 123863

Committed r105844: <http://trac.webkit.org/changeset/105844>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Kentaro Hara]

Adam, darin: This might be a hack (or might be still wrong:-), but how about the following idea?

==== Current behavior ====

supplemental_dependency.tmp : $(ALL_IDL_FILES)
    perl resolve-supplemental.pl $(ALL_IDL_FILES)   # resolve-supplemental.pl generates supplemental_dependency.tmp.

JS%.h : %.idl supplemental_dependency.tmp
    perl generate-bindings.pl supplemental_dependency.tmp $<    # generate-bindings.pl generates JS%.h and JS%.cpp.


==== New behavior ====

supplemental_dependency.tmp : $(ALL_IDL_FILES)
    perl resolve-supplemental.pl $(ALL_IDL_FILES)   # resolve-supplemental.pl generates supplemental_dependency.tmp.

supplemental_dependency_with_effective_timestamp.tmp : supplemental_dependency.tmp
    cp ... # copy supplemental_dependency.tmp to supplemental_dependency_with_effective_timestamp.tmp only when the bytes differ.

JS%.h : %.idl supplemental_dependency_with_effective_timestamp.tmp
    perl generate-bindings.pl supplemental_dependency_with_effective_timestamp.tmp $<    # generate-bindings.pl generates JS%.h and JS%.cpp.

[Adam Barth]

So, the copy rule will always fire, but we ought not worry because it will be fast?

[Adam Barth]

Isn't there a problem if I touch the contents of DOMWindowWebSockets.idl ?  supplemental_dependency_with_effective_timestamp.tmp won't change but JSDOMWindow.cpp won't be re-built.

[Kentaro Hara]

(In reply to comment #9)
> So, the copy rule will always fire, but we ought not worry because it will be fast?

Yes. Some command must always fire anyway. So the basic idea is to make the command faster.

(In reply to comment #10)
> Isn't there a problem if I touch the contents of DOMWindowWebSockets.idl ?  supplemental_dependency_with_effective_timestamp.tmp won't change but JSDOMWindow.cpp won't be re-built.

Ah, right. More simple, a problem happens when we just touch Element.idl. supplemental_dependency_with_effective_timestamp.tmp won't change, and JSElement.cpp won't be re-built.

The problem here is that supplemental_dependency_with_effective_timestamp.tmp is not updated when the update is required. Then how about adding a hash (e.g. MD5) of each IDL to supplemental_dependency.tmp, like this?

    A.idl (r843wqrdjwaoufe)
    B.idl (ru3oafjslajfe) B_supplemental.idl (fjdskafeiuwo32) B_Supplemental2.idl (r4owafnsfdlkre)
    C.idl (r483qokesafsda)

[Adam Barth]

Here's another idea;

1) Whenever any IDL file changes, run resolve-supplemental, and for each line in supplemental_dependency.tmp, if the main IDL file is older than any of the supplemental IDL files, touch the main IDL file.
2) Have JSFoo.cpp depend only on JSFoo.idl (the main IDL file).

Thoughts?

[Darin Adler]

(In reply to comment #12)
> Here's another idea;
> 
> 1) Whenever any IDL file changes, run resolve-supplemental, and for each line in supplemental_dependency.tmp, if the main IDL file is older than any of the supplemental IDL files, touch the main IDL file.
> 2) Have JSFoo.cpp depend only on JSFoo.idl (the main IDL file).
> 
> Thoughts?

Here’s another version of this that doesn’t rely on touching source files. Maybe not worded quite correctly:

1) For each main IDL file, have a make rule that creates/touches a corresponding .cpp-needs-to-be-gnerated file.

2) For each supplemental IDL file, have a make rule that createes/touches the main .cpp-needs-to-be-gnerated files as well as whatever file is generated to track the fact that the supplemental file was processed. This need not be done all at once with resolve-supplemental, or could be done for each supplemental file individually. There is no order dependency between this and (1).

3) In a separate pass (needs to be after [2], yet not dependent on [2]), have JSFoo.cpp depend on JSFoo.idl-needs-compliation.

[Kentaro Hara]

(In reply to comment #12)
> Here's another idea;
> 
> 1) Whenever any IDL file changes, run resolve-supplemental, and for each line in supplemental_dependency.tmp, if the main IDL file is older than any of the supplemental IDL files, touch the main IDL file.
> 2) Have JSFoo.cpp depend only on JSFoo.idl (the main IDL file).
> 
> Thoughts?

Thanks! I might be misunderstanding, but you mean something like this?

supplemental_dependency.tmp : $(ALL_IDL_FILES)
    perl resolve-supplemental.pl $(ALL_IDL_FILES)   # resolve-supplemental.pl generates supplemental_dependency.tmp. It also touches main IDLs that must be updated.

JS%.cpp : %.idl
    perl generate-bindings.pl supplemental_dependency.tmp $<    # generate-bindings.pl generates JS%.h and JS%.cpp.

In this case, the order dependency between generate-bindings.pl and resolve-supplemental.pl does not appear in the makefile. How can we confirm that generate-bindings.pl runs after resolve-supplemental.pl?

# I am looking into darin's version.

[Kentaro Hara]

(In reply to comment #13)
> (In reply to comment #12)
> Here’s another version of this that doesn’t rely on touching source files. Maybe not worded quite correctly:
> 
> 1) For each main IDL file, have a make rule that creates/touches a corresponding .cpp-needs-to-be-gnerated file.
> 
> 2) For each supplemental IDL file, have a make rule that createes/touches the main .cpp-needs-to-be-gnerated files as well as whatever file is generated to track the fact that the supplemental file was processed. This need not be done all at once with resolve-supplemental, or could be done for each supplemental file individually. There is no order dependency between this and (1).
> 
> 3) In a separate pass (needs to be after [2], yet not dependent on [2]), have JSFoo.cpp depend on JSFoo.idl-needs-compliation.

Thanks, darin! I might be misunderstanding your idea, but you mean something like this?

supplemental_dependency.tmp : $(ALL_IDL_FILES)
    perl resolve-supplemental.pl $(ALL_IDL_FILES)   # Generates supplemental_dependency.tmp.

%.cpp-needs-to-be-generated : %.idl supplemental_dependency.tmp
    perl touch-cpp-needs-to-be-generated.pl $<    # touches %.cpp-needs-to-be-generated and all *.cpp-needs-to-be-generated s that %.idl is supplementing.

JS%.cpp : %.cpp-needs-to-be-generated
    perl generate-bindings.pl supplemental_dependency.tmp $<    # Generates JS%.h and JS%.cpp.


In this case, how can we guarantee that generate-bindings.pl for JSxxxx.cpp runs after xxxx.cpp-needs-to-be-generated is touched by all touch-cpp-needs-to-be-generated.pl s that may touch xxxx.cpp-needs-to-be-generated?

[Kentaro Hara]

I guess that we need to guarantee that generate-bindings.pl for each IDL must run after all supplemental dependencies are resolved. This implies that JS%.cpp must (directly or indirectly) depend on a file that describes all the supplemental dependencies (i.e. supplemental_dependency.tmp) as a explicit rule in makefile, like this:

    JS%.cpp : %.idl supplemental_dependency.tmp
        perl generate-bindings.pl ...

In other words, the following rules won't work as expected:

    JS%.cpp : %.idl
        perl generate-bindings.pl ...

or

    JS%.cpp : %.cpp-needs-to-be-generated
        perl generate-bindings.pl ...


=====================

Is the hash idea too hacky?

supplemental_dependency.tmp : $(ALL_IDL_FILES)
    perl resolve-supplemental.pl $(ALL_IDL_FILES)   # Generates supplemental_dependency.tmp, with a hash of each IDL.

supplemental_dependency_with_effective_timestamp.tmp : supplemental_dependency.tmp
    cp ... # copy supplemental_dependency.tmp to supplemental_dependency_with_effective_timestamp.tmp only when the bytes differ.

JS%.cpp : %.idl supplemental_dependency_with_effective_timestamp.tmp
    perl generate-bindings.pl supplemental_dependency_with_effective_timestamp.tmp $<    # Generates JS%.h and JS%.cpp.

[Kentaro Hara]

(In reply to comment #16)
> I guess that we need to guarantee that generate-bindings.pl for each IDL must run after all supplemental dependencies are resolved. This implies that JS%.cpp must (directly or indirectly) depend on a file that describes all the supplemental dependencies (i.e. supplemental_dependency.tmp) as a explicit rule in makefile, like this:
> 
>     JS%.cpp : %.idl supplemental_dependency.tmp
>         perl generate-bindings.pl ...
> 
> In other words, the following rules won't work as expected:
> 
>     JS%.cpp : %.idl
>         perl generate-bindings.pl ...
> 
> or
> 
>     JS%.cpp : %.cpp-needs-to-be-generated
>         perl generate-bindings.pl ...
> 
> 
> =====================
> 
> Is the hash idea too hacky?
> 
> supplemental_dependency.tmp : $(ALL_IDL_FILES)
>     perl resolve-supplemental.pl $(ALL_IDL_FILES)   # Generates supplemental_dependency.tmp, with a hash of each IDL.
> 
> supplemental_dependency_with_effective_timestamp.tmp : supplemental_dependency.tmp
>     cp ... # copy supplemental_dependency.tmp to supplemental_dependency_with_effective_timestamp.tmp only when the bytes differ.
> 
> JS%.cpp : %.idl supplemental_dependency_with_effective_timestamp.tmp
>     perl generate-bindings.pl supplemental_dependency_with_effective_timestamp.tmp $<    # Generates JS%.h and JS%.cpp.

Adam, Darin: I would like to fix the issue somehow before starting WebKit modularization. If the hash idea would be acceptable, I am happy to implement it. Or I would like to look for another idea based on the ideas you suggested above.

[Adam Barth]

> Adam, Darin: I would like to fix the issue somehow before starting WebKit modularization. If the hash idea would be acceptable, I am happy to implement it. Or I would like to look for another idea based on the ideas you suggested above.

The approach in Comment #17 will still cause all the CPP files to be regenerated whenever any IDL file changes.

I thought about this for a while but I couldn't come up with anything great.  I think my make foo isn't strong enough.  The thing I don't know how to do is to make one thing run after another but not be dependent on it.  Is there an idiom in Make for doing that?  If so, Darin's approach from Comment #13 seems fine.

[Darin Adler]

(In reply to comment #18)
> The thing I don't know how to do is to make one thing run after another but not be dependent on it.  Is there an idiom in Make for doing that?

The idiom is to put a phony target in the list of "all" dependencies before $(JS_DOM_HEADERS). That operation will run before the JS%.h rules runs. The order of dependencies matters.

[Kentaro Hara]

(In reply to comment #18)
> > Adam, Darin: I would like to fix the issue somehow before starting WebKit modularization. If the hash idea would be acceptable, I am happy to implement it. Or I would like to look for another idea based on the ideas you suggested above.
> 
> The approach in Comment #17 will still cause all the CPP files to be regenerated whenever any IDL file changes.

Thanks Adam, now I got it. A dependency DAG of Makefile is created statically (i.e. before any rule is fired)... Specifically,

B : A
    X

C : B
    Y

Even if X does not update B, Y is fired.

> The thing I don't know how to do is to make one thing run after another but not be dependent on it.  Is there an idiom in Make for doing that?  If so, Darin's approach from Comment #13 seems fine.

Yeah. Another problem is whether qmake, cmake and GYP also have the similar idiom.

Another idea is to use "recursive make", i.e. generate the required dependency dynamically. But I am not sure if it is possible in qmake, cmake and even GYP.

[Kentaro Hara]

(In reply to comment #19)
> (In reply to comment #18)
> > The thing I don't know how to do is to make one thing run after another but not be dependent on it.  Is there an idiom in Make for doing that?
> 
> The idiom is to put a phony target in the list of "all" dependencies before $(JS_DOM_HEADERS). That operation will run before the JS%.h rules runs. The order of dependencies matters.

Thanks Darin, maybe more modest way is "order-only-prerequisites":
http://www.gnu.org/software/make/manual/make.html#Prerequisite-Types

I guess this is what we need. I'll try Darin's idea using "order-only-prerequisites".

But I am not still sure the similar idiom is available in qmake, cmake and GYP.

[Kentaro Hara]

(In reply to comment #19)
> (In reply to comment #18)
> > The thing I don't know how to do is to make one thing run after another but not be dependent on it.  Is there an idiom in Make for doing that?
> 
> The idiom is to put a phony target in the list of "all" dependencies before $(JS_DOM_HEADERS). That operation will run before the JS%.h rules runs. The order of dependencies matters.

Darin: Sorry if I am misunderstanding your idea, but I am afraid that the phony target won't work well if make runs in parallel. Here are experimental results. 

#### test.mk from here ####
all: phony A.cpp B.cpp

dependency.txt: A.idl B.idl
        touch $@

%.touch: %.idl dependency.txt
        touch $@

%.cpp: %.touch
        touch $@

.PHONY: phony
phony: A.touch B.touch
        sleep 5
        echo "sleep end"
#### test.mk to here ####

#### The result from here ####
$ touch A.idl && make -j 2 -f test.mk
touch dependency.txt
touch A.touch
touch B.touch
touch A.cpp
sleep 5
touch B.cpp
echo "sleep end"
sleep end
#### The result to here ####

As you can see, "touch A.cpp" and "touch B.cpp" are fired without waiting for the phony.

Below is the result of order-only-prerequisites.

#### test2.mk from here ####
all: A.cpp B.cpp

dependency.txt: A.idl B.idl
        touch $@

A.touch: A.idl dependency.txt
        touch $@

B.touch: B.idl dependency.txt
        sleep 5
        touch $@

%.cpp: %.touch | A.touch B.touch
        touch $@
#### test2.mk to here ####

#### The result from here ####
$ touch A.idl && make -j 2 -f test2.mk
touch dependency.txt
touch A.touch
sleep 5
touch B.touch
touch A.cpp
touch B.cpp
#### The result to here ####

As you can see, "touch A.cpp" and "touch C.cpp" are fired after waiting "touch A.cpp" and "touch B.cpp".

[Simon Fraser (smfr)]

If there's work that remains here, this bug needs to be reopened, or a new one filed. If reusing this bug, please fix the title.

[Kentaro Hara]

Let me reopen this bug as a meta bug to track the issue.

[Darin Adler]

(In reply to comment #22)
> I am afraid that the phony target won't work well if make runs in parallel.

Good point. I think we have solved this in the past with multiple makefiles. I’ll let you know if I think of anything more elegant.

[Kentaro Hara]

(In reply to comment #25)
> Good point. I think we have solved this in the past with multiple makefiles. I’ll let you know if I think of anything more elegant.

Thanks! For now I am trying to make a patch using order-only-prerequisites in DerivedSources.make for AppleWebKit.

[Kentaro Hara]

Darin, Adam: I implemented Darin's idea in AppleWebKit, but it did not work...

supplemental_dependency.tmp : $(ALL_IDLS)
    perl resolve-supplemental.pl $(ALL_IDLS)   # Generates supplemental_dependency.tmp

%.idl-needs-rebuild : %.idl supplemental_dependency.tmp
    perl update-idl-needs-rebuild.pl $< $@    # Updates %.idl-needs-rebuild only if the dependency about %.idl is changed

JS%.cpp : %.idl-needs-rebuild | *.idl-needs-rebuild
    perl generate-bindings.pl supplemental_dependency.tmp $<    # Generates JS%.cpp


Please consider the case where we edit Element.idl.

(1) resolve-supplemental.pl generates supplemental_dependency.tmp
(2) update-idl-needs-rebuild.pl runs for all IDLs, especially for Element.idl, but it does not update Element.idl-needs-rebuild because the dependency about Element.idl is not changed
(3) Consequently, generate-bindings.pl does not run for Element.idl...

Maybe we can solve this issue by adding a "hash" of Element.idl to Element.idl-needs-rebuild, so that update-idl-needs-rebuild.pl can touch Element.idl-needs-rebuild when the dependency about Element.idl is changed *and* Element.idl itself is changed. However, if we use a hash, my previous hash idea would be simpler.


What I've been misunderstanding is the following point:

> A dependency DAG of Makefile is created statically (i.e. before any rule is fired)... Specifically,
> 
> B : A
>     X
> 
> C : B
>     Y
> 
> Even if X does not update B, Y is fired.

This is wrong. If X does not update B, then Y does not fire. But if so, I think that my previous hash idea might work well.

Any idea?

[Kentaro Hara]

Adam, Darin: Sorry for the looooong discussion.

I discussed with Adam Barth in IRC and reached the following trick:

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 file'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


But... I found that it might not work in parallel. The problem is that update-idl-needs-rebuild.pl can touch xxxx.idl-needs-rebuild that does not appear on the left hand side of the make rule, and in that case "| *.idl-needs-rebuild" does not guarantee that the update of xxxx.idl-needs-rebuild has finished.

For simplicity, consider the following example:

=========================
all : A B

X : foo
    sleep 3
    touch Y  # Touches a file that does not appear on the left hand side

Y : foo

A : X
    touch A

B : Y | X
    touch B
=========================

The result is as follows:

$ touch foo
$ make -j 2   # First try
sleep 3
touch Y
touch A
$ make -j 2   # Second try
sleep 3
touch Y
touch A
touch B
$ make -j 2   # Third try
sleep 3
touch Y
touch A
$ make -j 2   # Fourth try
sleep 3
touch Y
touch A
touch B

Thus, "touch B" is certainly executed after "touch Y", but "touch B" is not always executed. That's a problem...

Maybe can we use "recursive make"? (But in that case it will become more difficult to implement the same logic in GYP)

[Kentaro Hara]

Please ignore Comment #28, that observation was wrong.

[Fujii Hironori]

I think this problem is gone.