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.
Created attachment 298895 [details] [diff] [review] patch
Attachment #298895 - Flags: review?(treilly)
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)
Created attachment 300842 [details] [diff] [review] patch v2 Add define OVERRIDE_GLOBAL_NEW for linux and solaris.
Once this patch is pushed I will enable the buildslave for solaris bits.
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. >
Ah, ok then. Sounds good.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
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.
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).
Created attachment 314484 [details] [diff] [review] Don't enable global new as default We give this choice to the application who use MMGC.
Attachment #314484 - Flags: review?(stejohns)
Comment on attachment 314484 [details] [diff] [review] Don't enable global new as default okie dokie
Attachment #314484 - Flags: review?(stejohns) → review+
Status: REOPENED → RESOLVED
Last Resolved: 11 years ago → 11 years ago
Resolution: --- → FIXED
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.