Closed Bug 629668 Opened 13 years ago Closed 11 years ago

Don't rebuild IPDL files when not necessary

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla11

People

(Reporter: Ms2ger, Assigned: BenWa)

References

(Blocks 1 open bug)

Details

(Whiteboard: [buildfaster:?])

Attachments

(2 files, 9 obsolete files)

      No description provided.
Could you be more specific? IPDL already doesn't rewrite the IPDL files unless they have changed, even though it regenerates them internally on every export pass? Are you asking that IPDL be taught to calculate dependencies and avoid even the internal rebuild? That's hard, and might not be worth it.
To expand on comment 1, IPDL internally compiles everything on every invocation to avoid regenerating .h's/.cpp's if no .ipdl's changed.  Changing some of the .h's (like PContent*.h) can trigger rebuilding a large portion of the tree.  So far we've preferred the more precise dependency tracking provided by this scheme (more precise as compared to timestamp tracking) because the IPDL compiler itself isn't slow ... usually.

 In "normal" python environments the IPDL compiler goes through the .ipdl's at ~10/second.  The only real issue I've seen is in scratchbox environments, which have a ridiculously slow python binary.  I certainly don't want to change this compilation scheme just to work around scratchbox borkenness.
I have never seen ipdl files go by at 10/second.  It's closer to 2-3/second on my local machine.
very annoying problem... make every build_all much longer
(In reply to comment #2)
> The only real issue I've seen is in scratchbox environments, which
> have a ridiculously slow python binary.  I certainly don't want to change this
> compilation scheme just to work around scratchbox borkenness.

I should say, *something* about scratchbox makes the IPDL ridiculously slow.  I don't know that it's the python binary itself.
Ok, I see that in scratchbox by some reason we are using system installed python,
/scratchbox/tools/bin/misc_runner /usr/bin/python2.6 /usr/bin/python2.6 /home/romaxa/mozilla-central/config/pythonpath.py......

Scratchbox by default has only Python 2.3.4 (#1, Nov  8 2010, 09:32:39)
ah, and main reason is that we are running arm python with Qemu emulation, that is the reason why it so slow.
On my two year old Linux64 box the .ipdl files take about 10 seconds to generate.  They are re-generated even if I haven't touched a single file.  Multiply that by the number of browser builds done by developers per year, etc, etc.

> Are you asking that IPDL be taught to calculate dependencies and avoid
> even the internal rebuild? That's hard, and might not be worth it.

The broader question is:  are fast builds worth it?
See also bug 680534 and bug 680536.
(In reply to Ed Morley [:edmorley] from comment #9)
> See also bug 680534 and bug 680536.

Those aren't related to this.

I'm not really working on build system enhancements right now, somebody could feel free to take this.
Assignee: khuey → nobody
Status: ASSIGNED → NEW
I might be wrong but it seems to be fixed...
(In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #11)
> I might be wrong but it seems to be fixed...

No, it's not.  I just made it silent by default.  ;-)  (see bug 686507)
(In reply to Ehsan Akhgari [:ehsan] from comment #12)
> (In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #11)
> > I might be wrong but it seems to be fixed...
> 
> No, it's not.  I just made it silent by default.  ;-)  (see bug 686507)

Oh... I would not have done that. Hiding an issue is very often the best way to not have it fixed :(
(In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #13)
> (In reply to Ehsan Akhgari [:ehsan] from comment #12)
> > (In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #11)
> > > I might be wrong but it seems to be fixed...
> > 
> > No, it's not.  I just made it silent by default.  ;-)  (see bug 686507)
> 
> Oh... I would not have done that. Hiding an issue is very often the best way
> to not have it fixed :(

Well, I didn't see anybody rushing to fix it, and it made me mad every time that I saw it in my nice silent console.  So I took action, and did my best.  If I knew how to fix this bug, I would've.  ;-)
Speed up building ipc/. On my Mac it goes from 5 seconds to 0.6 seconds (x2 for universal). Bjacob got similar results on Linux. I'm sure in some settings the speed ups are even better.
Assignee: nobody → bgirard
Status: NEW → ASSIGNED
Attachment #578127 - Flags: review?(jones.chris.g)
Attachment #578127 - Attachment is obsolete: true
Attachment #578128 - Flags: review?(jones.chris.g)
Attachment #578127 - Flags: review?(jones.chris.g)
Sorry for bug spam, forgot to qref
Attachment #578128 - Attachment is obsolete: true
Attachment #578128 - Flags: review?(jones.chris.g)
Attachment #578129 - Flags: review?(jones.chris.g)
heh, on scartchbox this patch decrease build time up to couple of minutes...
Cool, I'm glad to hear that.
Comment on attachment 578129 [details] [diff] [review]
Faster incremental builds for all the people!

>diff --git a/ipc/ipdl/ipdl.py b/ipc/ipdl/ipdl.py

>+op.add_option('-m', '--dependencies', dest='emmitdependencies', default=0, action='store_const', const=1,
>+              help="Emmit Makefile dependencies for incremental rebuilds")
> 

"emit"

This wants to be a |default=False|, |action='store_true'| rule.

>+    if emmitdependencies == 1:

Nit: |if emitdependencies:|

>diff --git a/ipc/ipdl/ipdl/__init__.py b/ipc/ipdl/ipdl/__init__.py

> def genipdl(ast, outdir):
>-    return IPDLCodeGen().cgen(ast)
>+    IPDLCodeGen(tempfile).cgen(ast)
>+

I don't understand this change.  I'm also not sure how it doesn't
throw an undefined reference error.

>+
>+def genm(ast, dir, filename):
>+    tempfile = StringIO()
>+    DependGen(tempfile).mgen(ast)
>+    filename = dir + "/" + os.path.basename(filename) + ".depends"
>+    print "Outputing to " + filename
>+    writeifmodified(tempfile.getvalue(), filename)
> 

You don't necessarily need to writeifmodified() the .depends file,
since it's unconditionally read by the Makefile, and it might be
faster anyway to directly overwrite the .depends.  If you don't have
evidence that either way is faster, I'd prefer to remove the
writeifmodified().

> 
> def writeifmodified(contents, file):
>+    # Bug 629668
>+    # We need to always rewrite modified files for incremental rebuilds
>+    # to work correctly because makes wants all the file written to be
>+    # never then EVERY ipdl. We could also touch the file when there
>+    # are no changes but we don't really care about incremental builds
>+    # when modifying IPDL files.

If we're not meaningfully using writeifmodified() anymore (it looks
like we aren't), let's just remove it.

Nice patch :).  Would like to see the next version.
Attachment #578129 - Flags: review?(jones.chris.g)
Attached patch patch review (obsolete) — Splinter Review
Attachment #578129 - Attachment is obsolete: true
Attachment #580088 - Flags: review?(jones.chris.g)
Attachment #580088 - Flags: review?(jones.chris.g) → review+
Try run for 88f62e92a2f2 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=88f62e92a2f2
Results (out of 14 total builds):
    failure: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/b56girard@gmail.com-88f62e92a2f2
Try run for 9ad184100042 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=9ad184100042
Results (out of 14 total builds):
    failure: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/b56girard@gmail.com-9ad184100042
Try run for 121468338f95 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=121468338f95
Results (out of 145 total builds):
    exception: 1
    success: 126
    warnings: 11
    failure: 7
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/b56girard@gmail.com-121468338f95
I'm a bit confused by this error. Any python experts have an idea?

I assume python has short circuit evaluation, thus we shouldn't call
makedirs if the path already exists.

Traceback (most recent call last):
  File "/builds/slave/try-osx64/build/config/pythonpath.py", line 52, in <module>
    main(sys.argv[1:])
  File "/builds/slave/try-osx64/build/config/pythonpath.py", line 44, in main
    execfile(script, frozenglobals)
  File "/builds/slave/try-osx64/build/ipc/ipdl/ipdl.py", line 120, in <module>
    ipdl.gencxx(filename, ast, headersdir, cppdir)
  File "/builds/slave/try-osx64/build/ipc/ipdl/ipdl/__init__.py", line 76, in gencxx
    writetofile(tempfile.getvalue(), filename)
  File "/builds/slave/try-osx64/build/ipc/ipdl/ipdl/__init__.py", line 92, in writetofile
    os.path.exists(dir) or os.makedirs(dir)
  File "/tools/buildbot/bin/../lib/python2.6/os.py", line 157, in makedirs
    mkdir(name, mode)
OSError: [Errno 17] File exists: '_ipdlheaders/mozilla/ipc'
(In reply to Benoit Girard (:BenWa) from comment #25)
> I'm a bit confused by this error. Any python experts have an idea?
> 
> I assume python has short circuit evaluation, thus we shouldn't call
> makedirs if the path already exists.

Correct.

> Traceback (most recent call last):
>   File "/builds/slave/try-osx64/build/config/pythonpath.py", line 52, in
> <module>
>     main(sys.argv[1:])
>   File "/builds/slave/try-osx64/build/config/pythonpath.py", line 44, in main
>     execfile(script, frozenglobals)
>   File "/builds/slave/try-osx64/build/ipc/ipdl/ipdl.py", line 120, in
> <module>
>     ipdl.gencxx(filename, ast, headersdir, cppdir)
>   File "/builds/slave/try-osx64/build/ipc/ipdl/ipdl/__init__.py", line 76,
> in gencxx
>     writetofile(tempfile.getvalue(), filename)
>   File "/builds/slave/try-osx64/build/ipc/ipdl/ipdl/__init__.py", line 92,
> in writetofile
>     os.path.exists(dir) or os.makedirs(dir)
>   File "/tools/buildbot/bin/../lib/python2.6/os.py", line 157, in makedirs
>     mkdir(name, mode)
> OSError: [Errno 17] File exists: '_ipdlheaders/mozilla/ipc'

Do we have a race condition here?
os.makedirs will raise an OSError if the target directory exists. It is recommended to protect calls to os.makedirs with os.path.exists. But, since we could have parallel execution, you will need to do something like:

try:
  os.makedirs(path)
except OSError as ex
  if ex.errno != errno.EEXIST:
     raise ex
  # else directory already exists. silently ignore and continue
Thanks for the suggestion, pushed to try.
Try run for 0afdb4d54914 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=0afdb4d54914
Results (out of 14 total builds):
    failure: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/b56girard@gmail.com-0afdb4d54914
Try run for 0afdb4d54914 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=0afdb4d54914
Results (out of 14 total builds):
    failure: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/b56girard@gmail.com-0afdb4d54914
Try run for 0afdb4d54914 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=0afdb4d54914
Results (out of 14 total builds):
    failure: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/b56girard@gmail.com-0afdb4d54914
Try run for 0afdb4d54914 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=0afdb4d54914
Results (out of 14 total builds):
    failure: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/b56girard@gmail.com-0afdb4d54914
Try run for 0afdb4d54914 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=0afdb4d54914
Results (out of 14 total builds):
    failure: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/b56girard@gmail.com-0afdb4d54914
Try run for 0afdb4d54914 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=0afdb4d54914
Results (out of 14 total builds):
    failure: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/b56girard@gmail.com-0afdb4d54914
Depends on: 709618
I fail. There should be a colon in the except line:

try:
  os.makedirs(path)
except OSError as ex:
  if ex.errno != errno.EEXIST:
     raise ex
  # else directory already exists. silently ignore and continue
I tried it locally first but nothing happened since I didn't touch a .ipdl file. I also had to import errno.

It failed again because 'except XXX as ex:' appears to no be supported in our python version on tinderbox. *sigh*
You probably want |except XXX, ex:|
Try run is looking good. I should probably avoid Python for the next little while :)
"except as" was added in Python 2.6 (http://docs.python.org/release/2.6.7/reference/compound_stmts.html#except and http://docs.python.org/release/2.5.4/ref/try.html). I thought we had at least 2.6 everywhere. I guess not. Double sorry about the confusion.

Perhaps someone can explain to me why we ship Python 2.7 as part of the Windows Mozilla Build installer yet have Python 2.5 on our build infrastructure. That makes no sense to me.
Because the build infrastructure is not on the latest version of Mozilla Build.
Attached patch patch review (test fixes) (obsolete) — Splinter Review
This patch replaced a few |writeifmodified| I forgot and changed os.makedirs(dir) to be within a try, see Comment 26, Comment 27.
Attachment #580905 - Flags: review?(jones.chris.g)
Try run for b69dacdf36f1 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=b69dacdf36f1
Results (out of 208 total builds):
    success: 178
    warnings: 27
    failure: 3
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/b56girard@gmail.com-b69dacdf36f1
Try run for 4d6f1fd12970 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=4d6f1fd12970
Results (out of 36 total builds):
    exception: 5
    success: 12
    warnings: 6
    failure: 13
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/b56girard@gmail.com-4d6f1fd12970
Try run for b69dacdf36f1 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=b69dacdf36f1
Results (out of 208 total builds):
    success: 178
    warnings: 27
    failure: 3
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/b56girard@gmail.com-b69dacdf36f1
(In reply to Mozilla RelEng Bot from comment #44)
my apologies, this comment is ever so slightly different than comment 42 so the bot posted.  you should not see any more dupe posts from the releng bot
Attachment #580905 - Flags: review?(jones.chris.g) → review+
Keywords: checkin-needed
Attachment #580088 - Attachment is obsolete: true
https://hg.mozilla.org/integration/mozilla-inbound/rev/329274bcfd57
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
Whoops, pre-emptive FIXED, need to get back into inbound mode, after our spell in m-c :-)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/329274bcfd57
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
Depends on: 711549
Backed out until Bug 711549 is fixed.

https://hg.mozilla.org/mozilla-central/rev/3d74a7c6d71b
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I'm not sure what's going on in bug 711549 and I've got my hands tied with bug 598873. It would be great if someone can take a look.
Whiteboard: [buildfaster?]
Whiteboard: [buildfaster?] → [buildfaster:?]
Blocks: 755684
Sorry I haven't been able to debug the pymake problem that caused this patch to land. We should really try to land this. According to Comment 15 a year ago it saved 4 seconds for any build (including a non chance build). We have more IPDL files today so I expect this number to be higher.

Can anyone debug the pymake problem? We could even try relanding this if we have reason to believe pymake build has sufficiently changed that it might of fixed the problem.
Try uses pymake now so just push?
BenWa, I am not certain but I think this patch does not work anymore. I tried it 4 months ago and it failed because of something like we have several .ipdl files that include the same .ipdlh file, and then it pulls the dependency several times and make complains about it.

I think it was something like that. Anyway, try the patch again with a clobber build and it might fail before getting to the pymake problem.
I think ipdlh didn't exist when I wrote this patch. Likely I only need to handle the AST node for them and emit them to the deps file.
We should try to land this once bug 868536 lands. Moving IPDLSRCS to moz.build also solves the problem of having ipc/ipdl/Makefile.in reference other parts of the tree. In the new world, we just put a bunch of IPDL_SOURCES in moz.build files and a unified .mk with all of them is written out for ipc/ipdl/Makefile.in to consume. No more VPATH abuse and multiple file inclusion. \o/
Status: REOPENED → NEW
Depends on: 868536
Blocks: 892644
Updated to work with .ipdlh files.  Maybe pymake fixes have fixed other issues in
the interim.
Attachment #580905 - Attachment is obsolete: true
Comment on attachment 774221 [details] [diff] [review]
make ipdlc generate actual dependencies for its files

Review of attachment 774221 [details] [diff] [review]:
-----------------------------------------------------------------

::: ipc/ipdl/ipdl/mgen.py
@@ +13,5 @@
> +#
> +# The Original Code is mozilla.org code.
> +#
> +# Contributor(s):
> +#   Benoit Girard <bgirard@mozilla.com>

We should remove this license block
Despite that one working just fine on my Linux box and reducing the time taken
by a no-op make in ipc/ by 90%, it bombed on try.  Here's one that seems to
work as advertised--at least on my Linux box, we'll see about try:

https://tbpl.mozilla.org/?tree=Try&rev=445dab9cf371
Attachment #774221 - Attachment is obsolete: true
Comment on attachment 774292 [details] [diff] [review]
make ipdlc generate actual dependencies for its files

Seems to work OK on Windows.

I assert that trivial changes have been made to BenWa's original patch WRT the python and so cjones's r+ still holds.  Going to request glandium's approval on the Makefile.in changes, though.
Attachment #774292 - Flags: review?(mh+mozilla)
Comment on attachment 774292 [details] [diff] [review]
make ipdlc generate actual dependencies for its files

Adding implicit r+ from cjones.
Attachment #774292 - Flags: review+
Comment on attachment 774292 [details] [diff] [review]
make ipdlc generate actual dependencies for its files

Never mind!  This is not quite what we want.
Attachment #774292 - Flags: review?(mh+mozilla)
Attachment #774292 - Flags: review+
OK, the .depends files being generated by the last patches were completely screwy.
This patch does a much better job with the depends files, e.g. for BluetoothTypes.ipdlh:

../../ipc/ipdl/_ipdlheaders/mozilla/dom/bluetooth/BluetoothTypes.h: /home/froydnj/src/mozilla-central-official/dom/bluetooth/ipc/BluetoothTypes.ipdlh 
BluetoothTypes.cpp: /home/froydnj/src/mozilla-central-official/dom/bluetooth/ipc/BluetoothTypes.ipdlh 

and for PBluetooth.ipdl (somewhat cut down, but rest assured we do include all the
relevant protocol files):

../../ipc/ipdl/_ipdlheaders/mozilla/dom/bluetooth/PBluetooth.h: /home/froydnj/src/mozilla-central-official/dom/bluetooth/ipc/PBluetooth.ipdl ...more...
../../ipc/ipdl/_ipdlheaders/mozilla/dom/bluetooth/PBluetoothParent.h: /home/froydnj/src/mozilla-central-official/dom/bluetooth/ipc/PBluetooth.ipdl ...more...
../../ipc/ipdl/_ipdlheaders/mozilla/dom/bluetooth/PBluetoothChild.h: /home/froydnj/src/mozilla-central-official/dom/bluetooth/ipc/PBluetooth.ipdl ...more...
PBluetooth.cpp: /home/froydnj/src/mozilla-central-official/dom/bluetooth/ipc/PBluetooth.ipdl ...more...
PBluetoothParent.cpp: /home/froydnj/src/mozilla-central-official/dom/bluetooth/ipc/PBluetooth.ipdl ...more...
PBluetoothChild.cpp: /home/froydnj/src/mozilla-central-official/dom/bluetooth/ipc/PBluetooth.ipdl /home/froydnj/src/mozilla-central-official/dom/ipc/PBlob.ipdl ...more...

The header dependencies are constructed so as to match those in the .pp files generated
by GCC; ideally this would work the same on Windows, will have to check somehow.

Comments welcome.
Attachment #774292 - Attachment is obsolete: true
Attachment #774418 - Flags: review?(mh+mozilla)
Attachment #774418 - Flags: review?(bent.mozilla)
I'm not entirely sure this patch improves the situation; touching dom/bluetooth/ipc/BluetoothTypes.ipdlh triggers a rebuild of all the object files anyway.  It looks like this is due to the headers (e.g. PContent{Parent,Child}.cpp depend on PBluetooth.h, and I suspect lots of things depend on PContent*.h), but it might be more complicated.  The patch *does* make no-op builds for ipc/ipdl significantly faster, so maybe that's enough of a win.
Comment on attachment 774418 [details] [diff] [review]
make ipdlc generate actual dependencies for its files

Review of attachment 774418 [details] [diff] [review]:
-----------------------------------------------------------------

You need to write out an empty rule for each file that's listed as a dependency. e.g.

BluetoothTypes.h: BluetoothInput1.ipdl

BluetootInput1.ipdl:


This is needed because if you delete one of the input files (happens a lot when refactor and rename files), make is like "I need to ensure BluetoothInput1.ipdl is up to date." But, if there is no BluetoothInput1.ipdl file and no target to make it exist, make is like "I don't know how to make target BluetoothInput1.ipdl" and aborts. You work around this by defining an empty rule.
Comment on attachment 774418 [details] [diff] [review]
make ipdlc generate actual dependencies for its files

Review of attachment 774418 [details] [diff] [review]:
-----------------------------------------------------------------

::: ipc/ipdl/Makefile.in
@@ +40,5 @@
>  # which is why we don't have explicit .h/.cpp targets here
> +export:: $(CPPSRCS)
> +
> +# XXX: we really need to list more python deps here
> +$(CPPSRCS) : $(ALL_IPDLSRCS) $(srcdir)/ipdl.py

This is why touching 1 file will rebuild all the outputs. This rule is saying "every file in CPPSRCS depends on every file in ALL_IPDLSRCS."

There are ways to do this properly. I'm content with handling it in a follow-up bug: what you have is already a win for no-op builds. We'll fix it by the end of Q3 because this is on the build config quarterly goals list :)
Comment on attachment 774418 [details] [diff] [review]
make ipdlc generate actual dependencies for its files

Review of attachment 774418 [details] [diff] [review]:
-----------------------------------------------------------------

::: ipc/ipdl/Makefile.in
@@ +40,5 @@
>  # which is why we don't have explicit .h/.cpp targets here
> +export:: $(CPPSRCS)
> +
> +# XXX: we really need to list more python deps here
> +$(CPPSRCS) : $(ALL_IPDLSRCS) $(srcdir)/ipdl.py

In fact, I think you can just remove this dep on ALL_IPDLSRCS and use that variable instead of $^ in the command (which removes the need for filter-out). This should work fine because if the cpp files don't exist, they will be generated. If they do exist, the dependencies are in the dependencies file.

BTW, this rule is going to be executed several times in parallel when doing make -j<n> because make is going to try to build several of the CPPSRCS in parallel, running the command for each of them.

::: ipc/ipdl/ipdl/__init__.py
@@ +35,5 @@
>      it is not.'''
>      return TypeCheck().check(ast, errout)
>  
>  
> +def gencxx(emitdependencies, ipdlfilename, hlast, outheadersdir, outcppdir):

the emitdependencies argument would feel better last with a default value.

@@ +69,3 @@
>  
> +    if emitdependencies:
> +        filename = outcppdir + "/" + os.path.basename(ipdlfilename) + ".depends"

It would be better to place them next to other dependency files, with the same kind of name (.deps/file.ext.pp)

@@ +73,5 @@
>  
>  def genipdl(ast, outdir):
>      return IPDLCodeGen().cgen(ast)
>  
> +def writetofile(contents, file):

You're removing the single most important feature of this function that is to not write anything if the file exists and has the same content as what would be written. (That being said, we have a class to handle that: mozbuild.util.FileAvoidWrite)

That's another reason why touching one file would rebuild everything.

::: ipc/ipdl/ipdl/mgen.py
@@ +4,5 @@
> +
> +import sys
> +
> +from ipdl.cgen import CodePrinter
> +from ipdl.cxx.ast import TypeArray, Visitor

You're not using TypeArray.
Attachment #774418 - Flags: review?(mh+mozilla) → review-
Comment on attachment 774418 [details] [diff] [review]
make ipdlc generate actual dependencies for its files

Review of attachment 774418 [details] [diff] [review]:
-----------------------------------------------------------------

::: ipc/ipdl/ipdl/__init__.py
@@ +73,5 @@
>  
>  def genipdl(ast, outdir):
>      return IPDLCodeGen().cgen(ast)
>  
> +def writetofile(contents, file):

I didn't think it would be needed if we had proper dependencies. But then again we can save some time in the case that a dependency changed but it had no impact.
Comment on attachment 774418 [details] [diff] [review]
make ipdlc generate actual dependencies for its files

Review of attachment 774418 [details] [diff] [review]:
-----------------------------------------------------------------

This looks generally fine but I'm going to wait to see the final patch you work out with gps and glandium. If they're happy I probably will be too :)

> BluetoothTypes.cpp: /home/froydnj/src/mozilla-central-official/dom/bluetooth/ipc/BluetoothTypes.ipdlh 

Do our dependencies normally have absolute paths like this?
Attachment #774418 - Flags: review?(bent.mozilla)
Attachment #774418 - Flags: review-
Current patch.  Now doesn't regenerate cpps at all when an ipdl file is changed.
Also still has the issue that parallel makes run ipdl.py multiple times, which
is highly suboptimal.  Need to investigate running ipdl.py for individual .ipdl
files and a separate invocation for IPCMessageStart.h.
(In reply to Nathan Froyd (:froydnj) (AFK until 22 July) from comment #70)
> Need to investigate running ipdl.py for individual .ipdl
> files and a separate invocation for IPCMessageStart.h.

I am having problems making this approach work.  Declaring the dependencies when ipdlsrcs.mk is created is simple enough:

%.cpp %.h: .../%.ipdlh
    $(call run-ipdlc,$<)

(defined similarly for .ipdl files and all the files they generate).  This idiom avoids issues with running ipdl.py for every target in parallel make.  It does have the downside of making clobber builds--and maybe even small change builds--slower.  (This is because even compiling simple .ipdlh files tends to require parsing a good chunk of all the .ipdl files in the tree, so you're repeating the parsing work across N files now...)  Anyway, solutions to collect and use proper dependency information and make parallel makes not do insane amounts of work and not slow down the clobber/serial build case work correctly welcome.  (Pick any two?)  (I'm also not sure this GNU make idiom works correctly in pymake.)

But ipdl.py seems to have issues when processing single files and/or processing files in an order other than the one we use.  For instance, running:

    ipdl.py <necessary options> /path/to/DOMTypes.ipdlh

will fail with:

/home/froydnj/src/mozilla-central-official.git/dom/ipc/DOMTypes.ipdlh:0: error: field `optionalInputStreamParams' of struct `ParentBlobConstructorParams' has unknown type `OptionalInputStreamParams'

OptionalInputStreamParams is defined in ipc/glue/InputStreamParams.ipdlh.  However:

    ipdl.py <necessary options> /path/to/DOMTypes.ipdlh /path/to/InputStreamParams.ipdlh

fails the same way.  But reversing the order:

    ipdl.py <necessary options> /path/to/InputStreamParams.ipdlh /path/to/DOMTypes.ipdlh

works just fine.  (This last one is implicitly the order we use, btw).

Chris, do you have any insight into why the type-checking of these files would depend on the order in which we process the files?
Flags: needinfo?(cjones.bugs)
I do not recommend doing any of this in make. We are trying to move all of the export tier (which includes the IPDL compilation step) away from make entirely. Instead we should continue to run a single ipdl.py command line and build the dependency system into the IPDL compiler itself.
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #72)
> I do not recommend doing any of this in make. We are trying to move all of
> the export tier (which includes the IPDL compilation step) away from make
> entirely. Instead we should continue to run a single ipdl.py command line
> and build the dependency system into the IPDL compiler itself.

That'd be interesting.  So in today's world we'd keep the current |export:| invocation of ipdl.py, but make ipdl.py only write files if their dependencies had changed?

Isn't this essentially what the writeifmodified bits do?  Or am I misunderstanding?

I don't know that any changes in this area are worthwhile under the Big Ball of Yarn that is the web of the .ipdl dependency graph gets untangled somewhat.
> That'd be interesting.  So in today's world we'd keep the current |export:|
> invocation of ipdl.py, but make ipdl.py only write files if their
> dependencies had changed?
> 
> Isn't this essentially what the writeifmodified bits do?  Or am I
> misunderstanding?

That is what writeifmodified does, but it's not what I'm proposing exactly. I'm proposing an additional step of not *compiling* the IPDL files if any of the inputs hasn't changed.

Even if we can just avoid recompiling any IPDL if *no* IPDL changes were made (which is the common case for depend builds) that would help a lot.
Ah, OK; this patch implements timestamp checking of all the inputs vs. all the outputs
to determine if any work is necessary.  Feels a little hackish, but definitely speeds
up no-op builds on my machine (x86-64 Linux @ 2.66 GHz).

Before:

[froydnj@cerebro ipdl]$ time make -srj20

real	0m7.927s
user	0m7.748s
sys	0m0.144s

After:

[froydnj@cerebro ipdl]$ time make -srj20

real	0m0.542s
user	0m0.508s
sys	0m0.016s

I've flagged Ben for review, but if any of the python-wise folks want to jump in
with comments, I'd appreciate those too.
Attachment #774418 - Attachment is obsolete: true
Attachment #775126 - Attachment is obsolete: true
Attachment #779307 - Flags: review?(bent.mozilla)
Comment on attachment 779307 [details] [diff] [review]
make ipdl.py check timestamps to determine whether parsing/codegen is necessary

Review of attachment 779307 [details] [diff] [review]:
-----------------------------------------------------------------

This is a nice workaround. It would be nice to avoid the ipdl.py invocation if nothing has changed. But at 0.5s overhead (compared to 8), I'll take what you have here and we can optimize the Makefiles in a followup bug.
Comment on attachment 779307 [details] [diff] [review]
make ipdl.py check timestamps to determine whether parsing/codegen is necessary

Review of attachment 779307 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with this addressed:

::: ipc/ipdl/ipdl.py
@@ +60,5 @@
> +headersmap = {}
> +for (path, dirs, headers) in os.walk(headersdir):
> +    for h in headers:
> +        base = os.path.basename(h)
> +        headersmap[base] = os.path.join(path, h)

This will cause problems if we ever have two protocols with the same name in different namespaces... Could you abort and print a message here if headersmap[base] has something in it already?
Attachment #779307 - Flags: review?(bent.mozilla) → review+
(In reply to ben turner [:bent] from comment #77)
> Comment on attachment 779307 [details] [diff] [review]
> make ipdl.py check timestamps to determine whether parsing/codegen is
> necessary
> 
> Review of attachment 779307 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me with this addressed:
> 
> ::: ipc/ipdl/ipdl.py
> @@ +60,5 @@
> > +headersmap = {}
> > +for (path, dirs, headers) in os.walk(headersdir):
> > +    for h in headers:
> > +        base = os.path.basename(h)
> > +        headersmap[base] = os.path.join(path, h)
> 
> This will cause problems if we ever have two protocols with the same name in
> different namespaces... Could you abort and print a message here if
> headersmap[base] has something in it already?

Good point.  Will address with:

        base = os.path.basename(h)
        if base in
            root, ext = os.path.splitext(base)
            print >>sys.stderr, 'A protocol named', root, 'exists in multiple namespaces'
            sys.exit(1)
        headersmap[base] = os.path.join(path, h) 

(In reply to Gregory Szorc [:gps] from comment #76)
> This is a nice workaround. It would be nice to avoid the ipdl.py invocation
> if nothing has changed. But at 0.5s overhead (compared to 8), I'll take what
> you have here and we can optimize the Makefiles in a followup bug.

It's worth noting that the 0.5s overhead is for the entirety of |make|; a no-op |make export| is about 0.05s with this patch.
Few days (or month) back I see Google Chrome dev landing something similar bug fix in their Chromium snapshot, I bookmarked one of the relevant study link they did to speed up Blink IDL Compiler. Have a look at it, might be able to find some good information: https://docs.google.com/document/d/1AnPi-hprU7wiSd1jz4eyP5nPQgtLnFCalHC7kMJ9lXI/edit#

Don't mind my stupid comment...
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #72)
> I do not recommend doing any of this in make. We are trying to move all of
> the export tier (which includes the IPDL compilation step) away from make
> entirely. Instead we should continue to run a single ipdl.py command line
> and build the dependency system into the IPDL compiler itself.

I don't understand why we wouldn't still generate the .deps file. This will avoid invoking the compiler at all and works well with our current setup. We shouldn't be forking any processes if there's nothing to do. I'd much prefer seeing the build system understand the dependencies better. We can still have all the IPDL targets depend on all the IPDL source + deps and be built as the same target and benefit from the work done here.
(In reply to Benoit Girard (:BenWa) from comment #80)
> I don't understand why we wouldn't still generate the .deps file. This will
> avoid invoking the compiler at all and works well with our current setup. We
> shouldn't be forking any processes if there's nothing to do. I'd much prefer
> seeing the build system understand the dependencies better. We can still
> have all the IPDL targets depend on all the IPDL source + deps and be built
> as the same target and benefit from the work done here.

I hear this, but the problem so far has been:

1) |export| should depend on the .cpp/.h files.
2) The .cpp/.h files should depend on the appropriate IPDL file.

But then what do you write for the rules to regenerate the individual .cpp/.h files?  You shouldn't compile all the IPDL files for each set of files, because then you're invoking the IPDL compiler many many times for no real benefit.  And you can't say:

export:: $(CPPFILES)

$(CPPFILES) : ...
     $(PYTHON) $(srcdir)/ipdl.py ...

with deps for the individual CPPFILES because that also invokes the compiler many many times for no benefit in parallel makes.  (*maybe* you can do something clever with pattern rules on the left side, but I have no idea whether pymake supports that.)  You can't compile individual IPDL files because something in the typechecker doesn't work right (arguably a bug and should be fixed).

So dependency-driven recompilation doesn't work as things currently stand.  Checking for "no work needs to be done" is simple enough and doesn't require Makefile contortions to make it work for all cases.

Furthermore, the dependencies between the IPDL files themselves are so tangled that compiling a single file virtually requires loading all the IPDL files anyway.  I've attached a PNG of a graph of the dependencies between the IPDL files.  X -> Y indicates that X includes Y.

Note the prevalence of cycles.  The cycle-free outliers are in bluetooth and plugins, but I'm pretty sure nearly all of the graph is reachable from an arbitrary node.
I agree with everything you've said above. I think the IPDL rule should just be:

$(ALL_CPPS) :(or ::?) $(ALL_IPDL_AND_DEPENDENCY)
  $(PYTHON) $(srcdir)/ipdl.py ...

Thus we're telling Make to not run IPDL if no dependencies have changed. And we're telling make that if any IPDL dependencies have changed a single run of ipdl.py will update all the required targets. Then your patch above kicks in and only does work for the targets that changed.

We get:
- Make understands the dependencies and doesn't have to run ipdl.py needlessly
- ipdl.py will compile all files that have changed at once but no more thanks to your patch.
(In reply to Benoit Girard (:BenWa) from comment #82)
> I agree with everything you've said above. I think the IPDL rule should just
> be:
> 
> $(ALL_CPPS) :(or ::?) $(ALL_IPDL_AND_DEPENDENCY)
>   $(PYTHON) $(srcdir)/ipdl.py ...
> 
> Thus we're telling Make to not run IPDL if no dependencies have changed. And
> we're telling make that if any IPDL dependencies have changed a single run
> of ipdl.py will update all the required targets.

What you are writing in English is not the same thing you are writing in Makefile-ese.  When you write:

a.cpp b.cpp c.cpp: $(DEPS)
     do-something...

that is interpreted by make as:

a.cpp: $(DEPS)
     do-something...

b.cpp: $(DEPS)
     do-something...

c.cpp: $(DEPS)
     do-something...

so if you |make -j3|, you are running |do-something| multiple times.  That is suboptimal--especially if |do-something| rebuilds all three files anyway.  It is particularly suboptimal with all the IPDL files we have and |do-something| being ipdl.py.  I suppose you could argue that even with the multiple rules for rebuilding .cpp/.h files, in conjunction with the timestamp-checking in Python, then *some* of the runs of ipdl.py will exit quickly and not do any work.  I am unsure that it would perform well with parallel builds, though; I could easily forsee a slowdown of N, where N is the # of parallel jobs you are running.  The build system racing like this, even correctly, does not fill me with happiness.

There is not a great way to tell make "hey, if *any* of these files are out of date, you just need to run a *single* command and that will update all of them".  I invite you to read through:

http://www.gnu.org/software/automake/manual/html_node/Multiple-Outputs.html

for how to write a robust solution to this.  I think, however, that the build peers would shoot me (or I would shoot myself) if I wrote a patch to implement this scheme.
No particular insight, sounds like a bug.
Flags: needinfo?(cjones.bugs)
https://hg.mozilla.org/mozilla-central/rev/a1e66f461331
Status: NEW → RESOLVED
Closed: 12 years ago11 years ago
Resolution: --- → FIXED
Thanks everyone for work on this bug.

I understand there are still lingering concerns that things aren't super optimal. However, we did go from ~8s to ~0.5.

Unless you are a build peer, I wouldn't worry too much about that remaining 0.5s. It will be addressed in due time, likely sometime this quarter as part of refactoring the export tier (bug 892644).

I think the non-build folks have put in enough effort with make files and I encourage you to take a break and return to comfortable territory :)
Blocks: 907394
I think this recently regressed. IPDLs are now taking about ~11s to build:

$ time make ipdl
/Users/gps/src/firefox/obj-firefox.noindex/_virtualenv/bin/python /Users/gps/src/firefox/config/pythonpath.py \
	  -I/Users/gps/src/firefox/other-licenses/ply \
	  /Users/gps/src/firefox/ipc/ipdl/ipdl.py -v \
	  --outheaders-dir=_ipdlheaders \
	  --outcpp-dir=. \
	  -I/Users/gps/src/firefox/content/media/webspeech/synth/ipc -I/Users/gps/src/firefox/dom/bluetooth/ipc -I/Users/gps/src/firefox/dom/devicestorage -I/Users/gps/src/firefox/dom/indexedDB/ipc -I/Users/gps/src/firefox/dom/ipc -I/Users/gps/src/firefox/dom/mobilemessage/src/ipc -I/Users/gps/src/firefox/dom/network/src -I/Users/gps/src/firefox/dom/plugins/ipc -I/Users/gps/src/firefox/dom/src/storage -I/Users/gps/src/firefox/gfx/layers/ipc -I/Users/gps/src/firefox/hal/sandbox -I/Users/gps/src/firefox/ipc/glue -I/Users/gps/src/firefox/ipc/testshell -I/Users/gps/src/firefox/js/ipc -I/Users/gps/src/firefox/layout/ipc -I/Users/gps/src/firefox/netwerk/cookie -I/Users/gps/src/firefox/netwerk/ipc -I/Users/gps/src/firefox/netwerk/protocol/ftp -I/Users/gps/src/firefox/netwerk/protocol/http -I/Users/gps/src/firefox/netwerk/protocol/websocket -I/Users/gps/src/firefox/netwerk/protocol/wyciwyg -I/Users/gps/src/firefox/uriloader/exthandler -I/Users/gps/src/firefox/uriloader/prefetch \
	  /Users/gps/src/firefox/content/media/webspeech/synth/ipc/PSpeechSynthesis.ipdl /Users/gps/src/firefox/content/media/webspeech/synth/ipc/PSpeechSynthesisRequest.ipdl /Users/gps/src/firefox/dom/bluetooth/ipc/BluetoothTypes.ipdlh /Users/gps/src/firefox/dom/bluetooth/ipc/PBluetooth.ipdl /Users/gps/src/firefox/dom/bluetooth/ipc/PBluetoothRequest.ipdl /Users/gps/src/firefox/dom/devicestorage/PDeviceStorageRequest.ipdl /Users/gps/src/firefox/dom/indexedDB/ipc/IndexedDBParams.ipdlh /Users/gps/src/firefox/dom/indexedDB/ipc/PIndexedDB.ipdl /Users/gps/src/firefox/dom/indexedDB/ipc/PIndexedDBCursor.ipdl /Users/gps/src/firefox/dom/indexedDB/ipc/PIndexedDBDatabase.ipdl /Users/gps/src/firefox/dom/indexedDB/ipc/PIndexedDBDeleteDatabaseRequest.ipdl /Users/gps/src/firefox/dom/indexedDB/ipc/PIndexedDBIndex.ipdl /Users/gps/src/firefox/dom/indexedDB/ipc/PIndexedDBObjectStore.ipdl /Users/gps/src/firefox/dom/indexedDB/ipc/PIndexedDBRequest.ipdl /Users/gps/src/firefox/dom/indexedDB/ipc/PIndexedDBTransaction.ipdl /Users/gps/src/firefox/dom/ipc/DOMTypes.ipdlh /Users/gps/src/firefox/dom/ipc/PBlob.ipdl /Users/gps/src/firefox/dom/ipc/PBlobStream.ipdl /Users/gps/src/firefox/dom/ipc/PBrowser.ipdl /Users/gps/src/firefox/dom/ipc/PContent.ipdl /Users/gps/src/firefox/dom/ipc/PContentDialog.ipdl /Users/gps/src/firefox/dom/ipc/PContentPermissionRequest.ipdl /Users/gps/src/firefox/dom/ipc/PCrashReporter.ipdl /Users/gps/src/firefox/dom/ipc/PDocumentRenderer.ipdl /Users/gps/src/firefox/dom/ipc/PMemoryReportRequest.ipdl /Users/gps/src/firefox/dom/ipc/PTabContext.ipdlh /Users/gps/src/firefox/dom/mobilemessage/src/ipc/PMobileMessageCursor.ipdl /Users/gps/src/firefox/dom/mobilemessage/src/ipc/PSms.ipdl /Users/gps/src/firefox/dom/mobilemessage/src/ipc/PSmsRequest.ipdl /Users/gps/src/firefox/dom/mobilemessage/src/ipc/SmsTypes.ipdlh /Users/gps/src/firefox/dom/network/src/PTCPServerSocket.ipdl /Users/gps/src/firefox/dom/network/src/PTCPSocket.ipdl /Users/gps/src/firefox/dom/plugins/ipc/PBrowserStream.ipdl /Users/gps/src/firefox/dom/plugins/ipc/PPluginBackgroundDestroyer.ipdl /Users/gps/src/firefox/dom/plugins/ipc/PPluginIdentifier.ipdl /Users/gps/src/firefox/dom/plugins/ipc/PPluginInstance.ipdl /Users/gps/src/firefox/dom/plugins/ipc/PPluginModule.ipdl /Users/gps/src/firefox/dom/plugins/ipc/PPluginScriptableObject.ipdl /Users/gps/src/firefox/dom/plugins/ipc/PPluginStream.ipdl /Users/gps/src/firefox/dom/plugins/ipc/PPluginSurface.ipdl /Users/gps/src/firefox/dom/plugins/ipc/PStreamNotify.ipdl /Users/gps/src/firefox/dom/src/storage/PStorage.ipdl /Users/gps/src/firefox/gfx/layers/ipc/LayerTransaction.ipdlh /Users/gps/src/firefox/gfx/layers/ipc/LayersSurfaces.ipdlh /Users/gps/src/firefox/gfx/layers/ipc/PCompositable.ipdl /Users/gps/src/firefox/gfx/layers/ipc/PCompositor.ipdl /Users/gps/src/firefox/gfx/layers/ipc/PGrallocBuffer.ipdl /Users/gps/src/firefox/gfx/layers/ipc/PImageBridge.ipdl /Users/gps/src/firefox/gfx/layers/ipc/PLayer.ipdl /Users/gps/src/firefox/gfx/layers/ipc/PLayerTransaction.ipdl /Users/gps/src/firefox/hal/sandbox/PHal.ipdl /Users/gps/src/firefox/ipc/glue/InputStreamParams.ipdlh /Users/gps/src/firefox/ipc/glue/URIParams.ipdlh /Users/gps/src/firefox/ipc/testshell/PTestShell.ipdl /Users/gps/src/firefox/ipc/testshell/PTestShellCommand.ipdl /Users/gps/src/firefox/js/ipc/JavaScriptTypes.ipdlh /Users/gps/src/firefox/js/ipc/PJavaScript.ipdl /Users/gps/src/firefox/layout/ipc/PRenderFrame.ipdl /Users/gps/src/firefox/netwerk/cookie/PCookieService.ipdl /Users/gps/src/firefox/netwerk/ipc/NeckoChannelParams.ipdlh /Users/gps/src/firefox/netwerk/ipc/PNecko.ipdl /Users/gps/src/firefox/netwerk/ipc/PRemoteOpenFile.ipdl /Users/gps/src/firefox/netwerk/protocol/ftp/PFTPChannel.ipdl /Users/gps/src/firefox/netwerk/protocol/http/PHttpChannel.ipdl /Users/gps/src/firefox/netwerk/protocol/websocket/PWebSocket.ipdl /Users/gps/src/firefox/netwerk/protocol/wyciwyg/PWyciwygChannel.ipdl /Users/gps/src/firefox/uriloader/exthandler/PExternalHelperApp.ipdl /Users/gps/src/firefox/uriloader/prefetch/POfflineCacheUpdate.ipdl
Generated C++ headers will be generated relative to "_ipdlheaders"
Generated C++ sources will be generated in "."
PSpeechSynthesis.ipdl
checking types
PSpeechSynthesisRequest.ipdl
checking types
BluetoothTypes.ipdlh
checking types
PBluetooth.ipdl
checking types
PBluetoothRequest.ipdl
checking types
PDeviceStorageRequest.ipdl
checking types
IndexedDBParams.ipdlh
checking types
PIndexedDB.ipdl
checking types
PIndexedDBCursor.ipdl
checking types
PIndexedDBDatabase.ipdl
checking types
PIndexedDBDeleteDatabaseRequest.ipdl
checking types
PIndexedDBIndex.ipdl
checking types
PIndexedDBObjectStore.ipdl
checking types
PIndexedDBRequest.ipdl
checking types
PIndexedDBTransaction.ipdl
checking types
DOMTypes.ipdlh
checking types
PBlob.ipdl
checking types
PBlobStream.ipdl
checking types
PBrowser.ipdl
checking types
PContent.ipdl
checking types
PContentDialog.ipdl
checking types
PContentPermissionRequest.ipdl
checking types
PCrashReporter.ipdl
checking types
PDocumentRenderer.ipdl
checking types
PMemoryReportRequest.ipdl
checking types
PTabContext.ipdlh
checking types
PMobileMessageCursor.ipdl
checking types
PSms.ipdl
checking types
PSmsRequest.ipdl
checking types
SmsTypes.ipdlh
checking types
PTCPServerSocket.ipdl
checking types
PTCPSocket.ipdl
checking types
PBrowserStream.ipdl
checking types
PPluginBackgroundDestroyer.ipdl
checking types
PPluginIdentifier.ipdl
checking types
PPluginInstance.ipdl
checking types
PPluginModule.ipdl
checking types
PPluginScriptableObject.ipdl
checking types
PPluginStream.ipdl
checking types
PPluginSurface.ipdl
checking types
PStreamNotify.ipdl
checking types
PStorage.ipdl
checking types
LayerTransaction.ipdlh
checking types
LayersSurfaces.ipdlh
checking types
PCompositable.ipdl
checking types
PCompositor.ipdl
checking types
PGrallocBuffer.ipdl
checking types
PImageBridge.ipdl
checking types
PLayer.ipdl
checking types
PLayerTransaction.ipdl
checking types
PHal.ipdl
checking types
InputStreamParams.ipdlh
checking types
URIParams.ipdlh
checking types
PTestShell.ipdl
checking types
PTestShellCommand.ipdl
checking types
JavaScriptTypes.ipdlh
checking types
PJavaScript.ipdl
checking types
PRenderFrame.ipdl
checking types
PCookieService.ipdl
checking types
NeckoChannelParams.ipdlh
checking types
PNecko.ipdl
checking types
PRemoteOpenFile.ipdl
checking types
PFTPChannel.ipdl
checking types
PHttpChannel.ipdl
checking types
PWebSocket.ipdl
checking types
PWyciwygChannel.ipdl
checking types
PExternalHelperApp.ipdl
checking types
POfflineCacheUpdate.ipdl
checking types

real	0m10.158s
user	0m9.972s
sys	0m0.175s

I plan to just avoid the ipdl.py invocation altogether in bug 907394. But someone may want to look into why we're not spending 0.5s any more.
(In reply to Gregory Szorc [:gps] from comment #87)
> I think this recently regressed. IPDLs are now taking about ~11s to build:
>
> real	0m10.158s
> user	0m9.972s
> sys	0m0.175s
> 
> I plan to just avoid the ipdl.py invocation altogether in bug 907394. But
> someone may want to look into why we're not spending 0.5s any more.

Takes a half-second for me locally on an already built tree.  You sure you weren't timing that on a clobber-ish build?
I used the 2nd time from a local run!
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.