Closed Bug 1134565 Opened 9 years ago Closed 9 years ago

Angle uses system operator new

Categories

(Core :: Memory Allocator, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

Details

(Whiteboard: [MemShrink])

Attachments

(2 files)

A snarky (and misled) comment added in bug 774755 in libEGL/libGLESv2 Makefiles made its way through to angle's moz.build in bug 1010371, with the effective result of making angle not use mozalloc.h's operator new, and use the system one instead.

In practice, it means OOM will not abort(), but emit an exception, which fortunately should crash, but more importantly, on Windows (and fortunately Windows only), it means Angle likely doesn't allocate memory with jemalloc!

I'll note that neither of those patches were reviewed by a build system peer... I'll also note it's the third time bug 1010371 hits my radar, which, to me means something should be changed with the way angle is managed in mozilla-central.
(In reply to Mike Hommey [:glandium] from comment #0)
> A snarky (and misled) comment added in bug 774755 in libEGL/libGLESv2
> Makefiles made its way through to angle's moz.build in bug 1010371, with the
> effective result of making angle not use mozalloc.h's operator new, and use
> the system one instead.

Heh, missing a part here: The snarky comment was accompanying a DISABLE_STL_WRAPPING = True.
glandium++
FWIW, I intend to use the reviewer metadata being added in bug 1132771 to enable not only automatic reviewer selection, but also to power auditing for changes that don't get proper review. If all goes according to plan, the machines should be able to automatically flag patches not receiving adequate review before they land.
FWIW, I want to kill the use of generate_mozbuild.py for both skia and angle. It is possible to refer to gyp files from moz.build directly, and while the gyp processor doing that is currently laden with webrtc specific context, it should be possible to use it for angle and skia.

I however don't want to block fixing this bug on that, so I'll just fix generate_mozbuild.py and the in-tree moz.build for now.
This looks awkward, but the generated moz.builds for libEGL and libGLESv2 are byte-equivalent to the ones in-tree with this patch.

I'll leave it to you to land that in the angle repository.
Attachment #8566874 - Flags: review?(vladimir)
This is the result of running generate_mozbuild.py as modified by attachment 8566874 [details] [diff] [review], and gets us back to where we were wrt stl wrapping in angle before bug 1010371.
Attachment #8566875 - Flags: review?(gps)
Attachment #8566875 - Flags: review?(gps) → review+
Whiteboard: [MemShrink]
Comment on attachment 8566874 [details] [diff] [review]
Fix generate_mozbuild.py to not set DISABLE_STL_WRAPPING in gfx/angle

Please push to http://github.com/mozilla/angle.
Attachment #8566874 - Flags: checkin?(vladimir)
https://hg.mozilla.org/mozilla-central/rev/a279774a1f3b
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Attachment #8566874 - Flags: checkin?(vladimir)
Comment on attachment 8566875 [details] [diff] [review]
Remove DISABLE_STL_WRAPPING in gfx/angle

Sorry, got a bit mixed up there with the Gecko patch. Setting the checkin? flag annoying makes this show up on our bug queries :(
Attachment #8566875 - Flags: checkin?(vladimir)
Dan, can you please help upstream attachment 8566875 [details] [diff] [review]?
Flags: needinfo?(dglastonbury)
It's not even about upstreaming, it's about pushing to http://github.com/mozilla/angle.
Sure thing. I can push to gh/moz/angle.
Flags: needinfo?(dglastonbury)
Attachment #8566875 - Flags: checkin?(vladimir) → checkin+
You need to log in before you can comment on or make changes to this bug.