Closed
Bug 413804
Opened 16 years ago
Closed 16 years ago
Enable GCGlobalNew.cpp for sunstudio compiler.
Categories
(Tamarin Graveyard :: Garbage Collection (mmGC), defect)
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: leon.sha, Assigned: leon.sha)
References
Details
Attachments
(2 files, 1 obsolete file)
3.52 KB,
patch
|
stejohns
:
review+
|
Details | Diff | Splinter Review |
560 bytes,
patch
|
stejohns
:
review+
|
Details | Diff | Splinter Review |
Sunstudio is the default complier on solaris. Sunstudio do not support the always inline feature -- "inline __attribute__((always_inline)) ". So we should enable GCGlobalNew.cpp when using sunstudio. Aslo __hidden is required to avoid conflict with system new.
Attachment #298895 -
Flags: review?(treilly)
Comment 2•16 years ago
|
||
I don't see anything in this patch about always_inline, just __hidden. Does Sunstudio have *any* way to force a function inline?
(In reply to comment #2) > I don't see anything in this patch about always_inline, just __hidden. > > Does Sunstudio have *any* way to force a function inline? > We will not inline "new" when built with sunstudio. We use __hidden to make sure that we call our "new" instead of system "new". always_inline is in GCGlobalNew.h. There is no way to force a function inline in sunstudio. You can only suggest sunstudio to inline a function, but sunstudio does not guarantee the function is always inlined. Also I found the OVERRIDE_GLOBAL_NEW is not defined, while in flash player it is defined. Should I also add it into linuxbuild.h and solarisbuild.h?
Attachment #298895 -
Flags: review?(treilly) → review?(stejohns)
Add define OVERRIDE_GLOBAL_NEW for linux and solaris.
Attachment #298895 -
Attachment is obsolete: true
Attachment #300842 -
Flags: review?(stejohns)
Attachment #298895 -
Flags: review?(stejohns)
Comment 5•16 years ago
|
||
Once this patch is pushed I will enable the buildslave for solaris bits.
Comment 6•16 years ago
|
||
Looks good, except I'm unclear as to why #define OVERRIDE_GLOBAL_NEW was added for Linux as well as Solaris.
I have checked the flash player souce. When building flash player, OVERRIDE_GLOBAL_NEW was always added as a compiler option for linux. I thought it should be a default setting. If it is not, I need to redo the patch. (In reply to comment #6) > Looks good, except I'm unclear as to why > > #define OVERRIDE_GLOBAL_NEW > > was added for Linux as well as Solaris. >
Comment 8•16 years ago
|
||
Ah, ok then. Sounds good.
Updated•16 years ago
|
Attachment #300842 -
Flags: review?(stejohns) → review+
Pushed.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment 10•16 years ago
|
||
Heh--that OVERRIDE_GLOBAL_NEW broke ActionMonkey on Linux. (Solaris too, probably, fwiw.) This is because some Mozilla source files are including MMgc.h, indirectly, but don't use it (not intentionally, anyway) and thus aren't linked against it. Bug 427828.
Comment 11•16 years ago
|
||
Leon, can we revert the OVERRIDE_GLOBAL_NEW change and reopen this bug? Flash Player isn't the only application using MMgc anymore. I think it makes sense for the default setting to be the same on all platforms (especially since there's no way to override this and turn it off).
Assignee | ||
Comment 12•16 years ago
|
||
We give this choice to the application who use MMGC.
Attachment #314484 -
Flags: review?(stejohns)
Comment 13•16 years ago
|
||
Comment on attachment 314484 [details] [diff] [review] Don't enable global new as default okie dokie
Attachment #314484 -
Flags: review?(stejohns) → review+
Assignee | ||
Comment 14•16 years ago
|
||
Pushed in.
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
Comment 15•15 years ago
|
||
Resolved fixed engineering / work item that has been pushed. Setting status to verified.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•