(not currently active) Ted Mielczarek Reporter
	Description • 17 years ago

shaver had a crazy scheme, and I have a patch, which I'll attach when my build finishes. This works on Win32/VC++, might need some tweaking for GCC.

	(not currently active) Ted Mielczarek Reporter
	Comment 1 • 17 years ago

Without:
real    27m37.750s

With:
real    21m51.250s

Built without -janything, just a straight |make| on a Core 2 Duo/2gb ram WinXP system.

	(not currently active) Ted Mielczarek Reporter
	Comment 2 • 17 years ago

With -j4, without the patch:
real    20m52.047s

With -j4, with the patch:
real    21m18.469s

Makes sense, since we're effectively serializing the C++ compilation phase.

	Mike Shaver (:shaver -- probably not reading bugmail closely)
	Comment 3 • 17 years ago

Yeah, without being able to ||ize dirs, we'll hit a wall there, though I suspect that we use less CPU with the patch even in the -j4 case, so it might still be a win on more-constrained boxes like the VM-hosted tboxes.

Nonetheless, I am sad.

	Mats Palmgren (inactive)
	Comment 4 • 17 years ago

FWIW, I tweaked the patch to work with gcc on Linux.

With -j4, without the patch:
real    18m

With -j4, with the patch:
real    30m

The PROG_SEPARATE_OBJS change was necessary for xremote:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/widget/src/xremoteclient/Makefile.in&rev=1.23&root=/cvsroot&mark=74-76#71

	Mike Shaver (:shaver -- probably not reading bugmail closely)
	Comment 5 • 17 years ago

(In reply to comment #4)
> With -j4, without the patch:
> real    18m
> 
> With -j4, with the patch:
> real    30m

:(

	(not currently active) Ted Mielczarek Reporter
	Comment 6 • 17 years ago

This would probably be better with bug 167254.

	(not currently active) Ted Mielczarek Reporter
	Comment 7 • 16 years ago

This is the magic+tweaks patch updated to trunk.

	(not currently active) Ted Mielczarek Reporter
	Comment 8 • 16 years ago

This is much better with my patch from bug 461395. See the data in bug 461395 comment 1.

	(not currently active) Ted Mielczarek Reporter
	Comment 10 • 16 years ago

Note to self: I should add something like mddepend.pl does to the deps for $OBJENTRY), so that if any of the object files are missing, it gets rebuilt.

	(not currently active) Ted Mielczarek Reporter
	Comment 11 • 16 years ago

So the data in bug 461395 suggests that this is nice on Windows (with the two patches combined), but break-even or a loss on other platforms. I'm going to tweak this patch so that we'll only enable it on Windows. In fact, the exact logic if going to be something like:

if building on Windows:
  if (makefile requests single-pass-compile) or (building with -j1):
    enable single-pass-compile

Since on Windows it looks to be always a win in the -j1 case, we should enable it there. Otherwise, directories will have to opt-in, since they'll only see a win if they get PARALLEL_DIRS. I'll make content/ use this since it's clearly a win there.

	Brad Lassey [:blassey] (use needinfo?)
	Comment 12 • 15 years ago

without this patch, building Fennec for WinMo:
real    85m42.050s
user    3m9.826s
sys     8m33.697s

with this patch, building Fennec for WinMo:
real    61m52.327s
user    2m46.921s
sys     7m26.657s

ummm....can we land this now?

Ted, one note I deleted the line with "OBJENTRY = OBJENTRY.BOGUS" because it was breaking the build.

	Mitchell Field [:Mitch]
	Comment 13 • 15 years ago

This is simply a better patch. ted made some improvements and I tacked on the use of MSVC9+'s /MP compiler flag. (http://msdn.microsoft.com/en-us/library/bb385193.aspx)

Needs the patch from bug 538606 applied as well.

	Mitchell Field [:Mitch]
	Comment 14 • 15 years ago

I had to revert the _VPATH_SRCS changes to fix bustage. It seems to work fine regardless, but I can't vouch for its correctness.

	(not currently active) Ted Mielczarek Reporter
	Comment 15 • 15 years ago

Fix a WinCE bug, I missed patching a link command line in the $(PROGRAM) link rule.

	(not currently active) Ted Mielczarek Reporter
	Comment 16 • 15 years ago

This is still breaking the WinMo builds on try server (but not the WinCE builds). blassey thinks his patch in bug 533542 might fix it. I've pushed a build with that patch + this patch to the try server to test.

	(not currently active) Ted Mielczarek Reporter
	Comment 17 • 15 years ago

This passed the try server when combined with bug 533542.

	neil@parkwaycc.co.uk
	Comment 18 • 15 years ago

If you have some way of knowing that you're -j1 then there seems to be an easier way of implementing this by adding a rule along these lines:

$(TARGETS):: $(CPPSRCS)
	$(ELOG) $(CCC) -c $(COMPILE_CXXFLAGS) $?

early on in the Makefile. (Without -j1 this won't compile the files quickly enough to stop the %.$(OBJ_SUFFIX) rules taking effect.) Either way though, I'm not sure how you expect dependency information to stay up-to-date.

	(not currently active) Ted Mielczarek Reporter
	Comment 19 • 15 years ago

I tried filtering MAKEFLAGS for -j1, but it apparently just doesn't work. I'm not sure what you mean about the dependency information, $(OBJENTRY) depends on $(CPPSRCS).

	neil@parkwaycc.co.uk
	Comment 20 • 15 years ago

(In reply to comment #19)
> I tried filtering MAKEFLAGS for -j1, but it apparently just doesn't work.
If you use -j1 then MAKEFLAGS won't have any j option at all. (Submakes run with -j and --jobserver-fds.)

> I'm not sure what you mean about the dependency information, $(OBJENTRY)
> depends on $(CPPSRCS).
So what about changes to headers?

	Mitchell Field [:Mitch]
	Comment 21 • 14 years ago

cl's -MP option conflicts with -showIncludes, the subject of bug 518136, so I've dropped the former option.

	Kyle Huey (Exited; not receiving bugmail, old account, do not use)
	Comment 22 • 14 years ago

I think we'll end up WONTFIXing this in favor of Bug 623617, but I'll leave this open for now.

	Mike Hommey [:glandium]
	Comment 23 • 9 years ago

I think we can reasonably WONTFIX this, considering bug 939583.