Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Fold js back into libxul

RESOLVED FIXED in mozilla36

Status

()

Core
JavaScript Engine
RESOLVED FIXED
7 years ago
3 years ago

People

(Reporter: khuey, Assigned: glandium)

Tracking

Trunk
mozilla36
x86
Windows 7
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(blocking2.0 -)

Details

Attachments

(3 attachments, 4 obsolete attachments)

We should undo the hack from http://hg.mozilla.org/mozilla-central/rev/0ff917ab21e4 before final if at all possible.
blocking2.0: --- → ?
Depends on: 611053
I threw this at tryserver last night and it worked with no troubles.
http://hg.mozilla.org/mozilla-central/rev/53952ab4a544
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Depends on: 592960
Backed out because this is triggering Bug 592960.

http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=94bcf27e5948
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla2.0b8 → ---
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Target Milestone: mozilla2.0b8 → ---
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Ftr,

(In reply to comment #2)
> http://hg.mozilla.org/mozilla-central/rev/53952ab4a544

http://hg.mozilla.org/comm-central/rev/7996fcc11af9
Port 'Bug 609976: Fold js back into libxul on Windows now that PGO seems less grumpy.' to comm-central to try and fix trace malloc bustage on windows.

(In reply to comment #3)
> http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=94bcf27e5948

http://hg.mozilla.org/comm-central/rev/5f253d5c2fb7
Back out changeset 7996fcc11af9 / porting of bug 609976 due to test failures in mozilla-central and the mozilla-central changeset being backed out
Flags: in-testsuite-
Version: unspecified → Trunk
Depends on: 614186

Updated

7 years ago
blocking2.0: ? → -

Comment 5

6 years ago
FWIW this is still building on try (http://tbpl.mozilla.org/?tree=Try&rev=592abbf5069e). Can we try landing it again?
I'd like to know that it does not bring back bug 592960 first.
We should investigate whether the omission of the PGO input files for mozjs (which was corrected for FF5) was the cause of bug 592960.

Comment 8

6 years ago
I tried pushing this to try last night (http://tbpl.mozilla.org/?tree=Try&rev=d973a8bf2e89) to get perf numbers and see if it regresses bug 592960, but to my surprise the build failed to compile at all (http://tinderbox.mozilla.org/showlog.cgi?log=Try/1306292468.1306312068.26873.gz).

Considering that this was working a few weeks ago, and that the size of xul.dll with statically linked JS is slightly over 17MB (in a local build using MSVC2010) now, I wonder if this is simply a hardcoded limit in MSVC2005 (ala bug 653662) rather than a compiler/linker bug.

If that's the case, this is yet another reason to switch to MSVC2010.

Comment 9

6 years ago
So it turns out the problem here is exactly the same as bug 653662. When we merge mozjs and libxul we go over the incremental linker limit and MSVC gives up compiling altogether.

Here's an all-green try-run of the patch in this bug with several JS reftest runs to make sure bug 592960 doesn't regress: http://tbpl.mozilla.org/?tree=Try&rev=6594e77dc7ba.

And compare-talos numbers: http://goo.gl/Np06A.

Comment 10

6 years ago
Created attachment 535833 [details] [diff] [review]
patch
Attachment #535833 - Flags: review?(khuey)
Comment on attachment 535833 [details] [diff] [review]
patch

This looks ok ... we could always check it in and see what happens.
Attachment #535833 - Flags: review?(khuey) → review+
Though, actually, this would turn PGO back on for JS, which we have explicitly turned off for the moment, so lets not check this in just yet.
Whiteboard: [Don't check in before figuring out JS PGO situation]

Comment 13

6 years ago
Created attachment 544387 [details] [diff] [review]
patch v2

This stopped working again for some reason. Since this is a likely a bug in MSVC 2005 I simply made the ENABLE_SHARED_JS hack check for the MSVC version as well. This patch also removes the PGO hack for JS (if that's ok) and re-enables incremental linking for debug builds using MSVC 2010.
Attachment #535833 - Attachment is obsolete: true
Attachment #544387 - Flags: review?(khuey)

Comment 14

6 years ago
Comment on attachment 544387 [details] [diff] [review]
patch v2

Please keep turning on JS PGO in a separate bug and patch, it messes up all our crash stats, and that for some of the most high-profile crashes, we don't want that to be mixed into other changes that might have an impact on crash signatures.

Comment 15

6 years ago
Created attachment 544392 [details] [diff] [review]
patch v3

This will need to depend on whatever bug decides to explicitly turn JS PGO back on then, as Kyle pointed out in comment 12.
Attachment #544387 - Attachment is obsolete: true
Attachment #544392 - Flags: review?(khuey)
Attachment #544387 - Flags: review?(khuey)
Comment on attachment 544392 [details] [diff] [review]
patch v3

Yes I want incremental linking back!

r=me.
Attachment #544392 - Flags: review?(khuey) → review+
This one we can check in now.

Comment 18

6 years ago
(In reply to comment #17)
> This one we can check in now.

That's probably not a good idea since it will mess with the results for bug 563319.
(In reply to comment #18)
> (In reply to comment #17)
> > This one we can check in now.
> 
> That's probably not a good idea since it will mess with the results for bug
> 563319.

Mmm, fair point.
Duplicate of this bug: 672946

Comment 21

6 years ago
I presume this can be landed now?
khuey, ping re comment 21.
Which part?  We can land the incremental linking part, but I'd rather hold off on the rest due to comment 18.
Incremental linking *would* be nice...
Yeah, lets do it.

Comment 26

6 years ago
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #25)
> Yeah, lets do it.

Can someone fold mozjs back this week?
(In reply to Taras Glek (:taras) from comment #26)
> (In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #25)
> > Yeah, lets do it.
> 
> Can someone fold mozjs back this week?

'it' is turning incremental linking back out.  We're not going to fold mozjs back in until we switch compilers.
s/out/on

Updated

6 years ago
Assignee: general → matjk7
Status: REOPENED → ASSIGNED
Depends on: 563318

Comment 29

6 years ago
Created attachment 568931 [details] [diff] [review]
patch v3

The incremental linking stuff is now bug 696627.
Attachment #544392 - Attachment is obsolete: true
Attachment #568931 - Flags: review+

Updated

6 years ago
Depends on: 709193

Updated

5 years ago
Depends on: 750661
n.b.: bug 673518 was fixed along the way.
Whiteboard: [Don't check in before figuring out JS PGO situation]
(Assignee)

Comment 31

3 years ago
Created attachment 8506075 [details] [diff] [review]
Fold mozjs.dll back into xul.dll

See bug 922912 comment 16.
Attachment #8506075 - Flags: review?(gps)
(Assignee)

Updated

3 years ago
Assignee: matjk7 → mh+mozilla
(Assignee)

Updated

3 years ago
Attachment #568931 - Attachment is obsolete: true
Attachment #8506075 - Flags: review?(gps) → review+
(Assignee)

Comment 32

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a758ca998666
This needed a clobber touch for Windows builds: https://hg.mozilla.org/integration/mozilla-inbound/rev/c2d6b4070a93

Updated

3 years ago
Depends on: 914596
https://hg.mozilla.org/mozilla-central/rev/a758ca998666
https://hg.mozilla.org/mozilla-central/rev/c2d6b4070a93
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
(Assignee)

Comment 35

3 years ago
Created attachment 8507274 [details] [diff] [review]
Followup to  - Build ICU as a shared library on windows
Attachment #8507274 - Flags: review?(mshal)

Updated

3 years ago
Attachment #8507274 - Flags: review?(mshal) → review+
(Assignee)

Comment 36

3 years ago
Created attachment 8507278 [details] [diff] [review]
Update removed-files.in to account for the removal of mozjs.dll
Attachment #8507278 - Flags: review?(mshal)

Updated

3 years ago
Attachment #8507278 - Flags: review?(mshal) → review+
(Assignee)

Comment 37

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/887c54a2dc05
https://hg.mozilla.org/integration/mozilla-inbound/rev/85fb6b5bc62a
(Assignee)

Comment 38

3 years ago
Followup to unbust windows:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cbe9f4f91599
(Assignee)

Comment 39

3 years ago
Backed out the removed-files.in part, as it appears it's not necessary anymore
https://hg.mozilla.org/integration/mozilla-inbound/rev/536c500f8e76
https://hg.mozilla.org/mozilla-central/rev/887c54a2dc05
https://hg.mozilla.org/mozilla-central/rev/cbe9f4f91599
Are these changes going to make it harder for us to revert back to MSVC 2010 if we need to?
Reverting would require this to be backed out, yes.
Depends on: 1086539
Blocks: 1098595
You need to log in before you can comment on or make changes to this bug.