Tell JS engine about malloc memory when binding object created

RESOLVED FIXED in Firefox 57

Status

()

enhancement
RESOLVED FIXED
2 years ago
4 months ago

People

(Reporter: jonco, Assigned: jonco)

Tracking

(Blocks 1 bug)

55 Branch
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

(Whiteboard: [qf])

Attachments

(1 attachment)

Following on from bug 1301863 we should inform the JS engine about malloc memory when binding objects are created for the other cases where we currently call JS_updateMallocMemory().

This should give the JS engine a better idea of how much malloc memory is tied to JS objects and allow for better GC scheduling decisions.
The patch does this for the classes CanvasRenderingContext2D and ImageBitmap.

I also changed the return type of BindingJSObjectMallocBytes() to size_t.
Attachment #8907110 - Flags: review?(amarchesini)
Comment on attachment 8907110 [details] [diff] [review]
bug1399141-update-malloc-count-on-binding

Review of attachment 8907110 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/canvas/ImageBitmap.cpp
@@ +2123,5 @@
> +
> +  // Calculate how many bytes are used.
> +  if (mData->GetFormat() == mozilla::ImageFormat::PLANAR_YCBCR) {
> +    return mData->AsPlanarYCbCrImage()->GetDataSize();
> +  } else if (mData->GetFormat() == mozilla::ImageFormat::NV_IMAGE) {

I know that this is old code, but, no "} else {" after a return. Just do:

if (mData->GetFormat() == mozilla::ImageFormat::PLANAR_YCBCR) {
  return mData->AsPlanarYCbCrImage()->GetDataSize();
}

if (mData->GetFormat() == mozilla::ImageFormat::NV_IMAGE) {
  return mData->AsNVImage()->GetBufferSize();
}

RefPtr<SourceSurface> surface = mData->GetAsSourceSurface();
const int bytesPerPixel = BytesPerPixel(surface->GetFormat());
return surface->GetSize().height * surface->GetSize().width * bytesPerPixel;
Attachment #8907110 - Flags: review?(amarchesini) → review+
Should we take it for FF57? Is it pretty safe? What is the perf impact?
Flags: needinfo?(jcoppeard)
No perf impact.  I think this is safe to take.
Flags: needinfo?(jcoppeard)
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff90282974ce
Attribute malloc memory when creating reflector object r=baku
https://hg.mozilla.org/mozilla-central/rev/ff90282974ce
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.