Store decoded images in VolatileBuffers

RESOLVED FIXED in mozilla30

Status

()

defect
RESOLVED FIXED
6 years ago
2 years ago

People

(Reporter: mwu, Assigned: mwu)

Tracking

(Blocks 1 bug)

unspecified
mozilla30
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:1.3T+, b2g-v1.3T fixed, b2g-v1.4 fixed)

Details

Attachments

(1 attachment, 21 obsolete attachments)

16.98 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
Assignee

Description

6 years ago
VolatileBuffers let us leave it to the OS to discard images.
Assignee

Comment 1

6 years ago
Posted patch Add gfxLockedImageSurface (obsolete) — Splinter Review
This is a gfxImageSurface that locks a VolatileBuffer while it exists. When the surface is destroyed, the VolatileBuffer is unlocked.
Assignee

Comment 2

6 years ago
This is pretty close - I think I just have to adjust the memory reporting and make sure tests pass.
Assignee

Comment 3

6 years ago
Posted patch Add gfxLockedImageSurface, v2 (obsolete) — Splinter Review
This makes gfxQuartzImageSurfaces compatible with gfxLockedImageSurfaces.
Attachment #8363786 - Attachment is obsolete: true
Assignee

Comment 4

6 years ago
This is actually pretty close to done, but it fails some reftests still..
Attachment #8363789 - Attachment is obsolete: true
Assignee

Comment 5

6 years ago
Posted patch Add gfxLockedImageSurface, v3 (obsolete) — Splinter Review
Updated for some API changes in VolatileBuffers
Attachment #8365590 - Attachment is obsolete: true
Assignee

Comment 6

6 years ago
Updated for VolatileBuffer API changes
Attachment #8365591 - Attachment is obsolete: true
Assignee

Comment 7

6 years ago
Fix build on debug builds.
Attachment #8366945 - Attachment is obsolete: true
Assignee

Comment 8

6 years ago
Attachment #8366948 - Attachment is obsolete: true
Assignee

Comment 9

6 years ago
Attachment #8368822 - Attachment is obsolete: true
Assignee

Comment 10

6 years ago
Since a new ImageContainer is generated whenever GetImageContainer is called on RasterImage, the current logic LayerTreeInvalidation will keep thinking the image has changed. (and cause certain reftests to time out) Here is one simple fix, probably wrong.
Assignee

Updated

6 years ago
Attachment #8368824 - Attachment description: Make imgFrame store images in VolatileBuffers, v4 (more context) → Make imgFrame store images in VolatileBuffers, v4 (wrong patch)
Attachment #8368824 - Attachment is obsolete: true
Assignee

Updated

6 years ago
Attachment #8368792 - Attachment description: Add gfxLockedImageSurface, v4 → [1/3] Add gfxLockedImageSurface, v4
Assignee

Updated

6 years ago
Attachment #8368859 - Attachment description: Make imgFrame store images in VolatileBuffers, v5 → [2/3] Make imgFrame store images in VolatileBuffers, v5
Assignee

Updated

6 years ago
Attachment #8368828 - Attachment description: Avoid unnecessary invalidation due to changing ImageContainers → [3/3] Avoid unnecessary invalidation due to changing ImageContainers
Assignee

Updated

6 years ago
Attachment #8368792 - Flags: review?(jmuizelaar)
Assignee

Comment 12

6 years ago
Comment on attachment 8368859 [details] [diff] [review]
[2/3] Make imgFrame store images in VolatileBuffers, v5

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

This makes imgFrame primarily hold on to image data via VolatileBuffers rather than gfxImageSurfaces. Whenever a surface is necessary, one is generated to wrap the VolatileBuffer as necessary. If the buffer was purged, RasterImage will start decoding again. ImageContainers are also generated as necessary, in order to avoid locking the buffer. This confuses the layer tree invalidation a bit. I have a few ideas on how to avoid that.

::: image/src/RasterImage.cpp
@@ +2119,5 @@
> +  if (GetNumFrames() > 0) {
> +    imgFrame *curframe = mFrameBlender.RawGetFrame(GetNumFrames() - 1);
> +    curframe->UnlockImageData();
> +  }
> +

The unlock is moved after Finish() since Finish tries to optimize the image. Unlocking the image here may remove access to the image surface.

::: image/src/imgFrame.cpp
@@ +837,5 @@
>  #endif
>  #ifdef XP_MACOSX
>    if (mQuartzSurface && aLocation == gfxMemoryLocation::IN_PROCESS_HEAP) {
> +    n += aMallocSizeOf(mQuartzSurface);
> +  }

This calculation seemed very wrong to me, so I fixed it while adding the size calculation for volatile buffers.

@@ +855,5 @@
> +    n += aMallocSizeOf(mVBuf);
> +    if (mVBuf->OnHeap()) {
> +      n += mVBuf->SizeOf(aMallocSizeOf);
> +    }
> +  }

This only counts the memory used by the surface if we're allocated on heap. We'll probably need to add more memory instrumentation for things stored in actual volatile buffers.
Attachment #8368859 - Flags: review?(jmuizelaar)
Assignee

Comment 13

6 years ago
I think this fix should be right.
Attachment #8368828 - Attachment is obsolete: true
Attachment #8369086 - Flags: review?(jmuizelaar)
Assignee

Comment 14

6 years ago
This uses GetSurface rather than LockImageSurface in the RasterImage::Draw path so we don't accidentally unoptimize images.
Attachment #8368859 - Attachment is obsolete: true
Attachment #8368859 - Flags: review?(jmuizelaar)
Attachment #8369094 - Flags: review?(jmuizelaar)
Assignee

Comment 15

6 years ago
Forgot to account for single color images in the last patch.
Attachment #8369094 - Attachment is obsolete: true
Attachment #8369094 - Flags: review?(jmuizelaar)
Attachment #8369198 - Flags: review?(jmuizelaar)
Assignee

Comment 16

6 years ago
I also have patches to make scaled images discardable and to make 16bit (RGB565) optimized images store in volatile buffers.
Assignee

Comment 17

6 years ago
Updated for VolatileBuffer API changes.
Attachment #8368792 - Attachment is obsolete: true
Attachment #8368792 - Flags: review?(jmuizelaar)
Attachment #8372658 - Flags: review?(jmuizelaar)
Assignee

Comment 18

6 years ago
Update for VolatileBuffer API changes.
Attachment #8369198 - Attachment is obsolete: true
Attachment #8369198 - Flags: review?(jmuizelaar)
Attachment #8372659 - Flags: review?(seth)
Attachment #8372659 - Flags: review?(jmuizelaar)
Comment on attachment 8372659 [details] [diff] [review]
[2/3] Make imgFrame store images in VolatileBuffers, v8

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

::: image/src/RasterImage.cpp
@@ +925,3 @@
>      frame->GetSurface(getter_AddRefs(framesurf));
> +    if (!framesurf && !frame->IsSinglePixel()) {
> +      // No reason to be optimized away here - the OS threw out the data

If there's a way you can assert that this is indeed the case, you should assert it. Murphy's Law suggests that down the road a bug will make us hit this pathway for some other reason.

@@ +928,5 @@
> +      if (!(aFlags & FLAG_SYNC_DECODE))
> +        return nullptr;
> +
> +      ForceDiscard();
> +      return GetFrame(aWhichFrame, aFlags);

You probably need to call CanForciblyDiscardAndRedecode() to verify it's safe to call ForceDiscard().

@@ +956,5 @@
>  
>    nsRefPtr<gfxASurface> imageSurface = GetFrame(FRAME_CURRENT, FLAG_NONE);
> +  if (!imageSurface) {
> +    // The OS threw out some or all of our buffer. Start decoding again.
> +    ForceDiscard();

Could also use an assertion and a call to CanForciblyDiscardAndRedecode() here.

@@ +1009,5 @@
>    *_retval = mImageContainer;
>    NS_ADDREF(*_retval);
> +  if (CanForciblyDiscardAndRedecode()) {
> +    mImageContainer = nullptr;
> +  }

Why are you calling CanForciblyDiscardAndRedecode() when you are neither discarding nor redecoding? (Well, you're not doing it here, at least. GetCurrentImage() might do it.)

@@ +2682,5 @@
> +  if (!frame->IsSinglePixel()) {
> +    frame->GetSurface(getter_AddRefs(surf));
> +    if (!surf) {
> +      // The OS threw out some or all of our buffer. Start decoding again.
> +      ForceDiscard();

Same comment as the two other ForceDiscard calls.
Assignee

Comment 20

6 years ago
Comment on attachment 8372659 [details] [diff] [review]
[2/3] Make imgFrame store images in VolatileBuffers, v8

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

::: image/src/RasterImage.cpp
@@ +928,5 @@
> +      if (!(aFlags & FLAG_SYNC_DECODE))
> +        return nullptr;
> +
> +      ForceDiscard();
> +      return GetFrame(aWhichFrame, aFlags);

The check is done by RasterImage::DecodingComplete. We only allow GetSurface to return null if

1. It's an indexed image. But we shouldn't hit the RasterImage::Draw path or this code in that case..
2. It's a single color/pixel image, which we check for here.
3. It's an image whose data got discarded by the OS. The data can only be discarded if there's no gfxLockedImageSurfaces holding on to it. We only allow imgFrame::mImageSurface to be set to null if we've previously determined that the image can forcibly discarded and redecoded. SetDiscardable is called on imgFrame if that is the case. mImageSurface can also be set to null by imgFrame::Optimize(), but in that case it is replaced by mOptSurface, so GetSurface would not return null.

@@ +1009,5 @@
>    *_retval = mImageContainer;
>    NS_ADDREF(*_retval);
> +  if (CanForciblyDiscardAndRedecode()) {
> +    mImageContainer = nullptr;
> +  }

If we can't discard and recode, then that means the imgFrame didn't get SetDiscardable() called on it and we don't need to be careful about releasing the image data back to the OS (by releasing all references to the gfxLockedImageSurface). More specifically, it lets us avoid reallocating the ImageContainer in cases where the image isn't discardable by the OS.

::: image/src/imgFrame.h
@@ +123,5 @@
> +#else
> +      return sur;
> +#endif
> +    }
> +    return nullptr;

This is probably a pretty good place to assert, since we should never return nullptr unless it's a single pixel optimized image, indexed image, or an OS discarded image.
I'll try to get to this review today.
Assignee

Comment 22

5 years ago
Unbitrotted, updated for the latest VolatileBuffer API update, and assertion added to ThebesSurface().
Attachment #8372659 - Attachment is obsolete: true
Attachment #8372659 - Flags: review?(seth)
Attachment #8372659 - Flags: review?(jmuizelaar)
Attachment #8374635 - Flags: review?(seth)
Attachment #8374635 - Flags: review?(jmuizelaar)
mwu,

Do you have any idea what the overhead is for VolatileBuffer::Lock() (in the case when the memory has not been purged) on the different platforms in number of cycles? The windows version looks like it could be heavy weight. Are memory maps being changed?
Flags: needinfo?(mwu)
Assignee

Comment 24

5 years ago
Hmm, I don't know what the overhead is. Looking at the ashmem implementation, it seems to run in constant time (in our case) and the implementation is relatively straightforward. On OSX/mach, the implementation has to mark every page for LRU purposes. The mapping is not changed on any platform after the VolatileBuffer is generated - the OS always uses the same address range.
Flags: needinfo?(mwu)
(In reply to Michael Wu [:mwu] from comment #24)
> Hmm, I don't know what the overhead is. Looking at the ashmem
> implementation, it seems to run in constant time (in our case) and the
> implementation is relatively straightforward. On OSX/mach, the
> implementation has to mark every page for LRU purposes. The mapping is not
> changed on any platform after the VolatileBuffer is generated - the OS
> always uses the same address range.

Do you mind measuring so we have some idea? i.e. just run with this patch and TimeStamp around the operation.
Assignee

Comment 26

5 years ago
BTW these patches don't anything on Windows since we start with a gfxWindowsSurface rather than VolatileBuffer + gfxLockedImageSurface. I do have a patch which removes the use of gfxWindowsSurface in imgFrame but I have no idea if that's a good idea or not.
Comment on attachment 8369086 [details] [diff] [review]
[3/3] Avoid unnecessary invalidation due to changing ImageContainers, v2

I don't think I'm the right person to review this. Including the imgIContainer in the ImageContainer doesn't feel great to me.
Attachment #8369086 - Flags: review?(jmuizelaar) → review?(roc)
(In reply to Michael Wu [:mwu] from comment #10)
> Since a new ImageContainer is generated whenever GetImageContainer is called
> on RasterImage,

But it isn't! The image container is cached in RasterImage::mImageContainer.
Assignee

Comment 29

5 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #28)
> (In reply to Michael Wu [:mwu] from comment #10)
> > Since a new ImageContainer is generated whenever GetImageContainer is called
> > on RasterImage,
> 
> But it isn't! The image container is cached in RasterImage::mImageContainer.

That's what the 2nd patch in this series does - it stops caching it so we don't pin the image down when we're not using it.
Comment on attachment 8372658 [details] [diff] [review]
[1/3] Add gfxLockedImageSurface, v5

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

I feel like it might be better to use SetData() api to preserve the lifetime of the the VolatileBufferPtr. That way we don't have to do the weird gymnastics in gfxQuartzImageSurface. This will also make it easier to port Moz2D because we won't need a dependency on the VolatileBuffer stuff.
Attachment #8372658 - Flags: review?(jmuizelaar) → review-
(In reply to Michael Wu [:mwu] from comment #20)
> The check is done by RasterImage::DecodingComplete. We only allow GetSurface
> to return null if

This reasoning is fine, but right now you've documented in a comment what it seems to me you could just as well document *and enforce* with an assertion. Complex invariants will not remain true over time unless we have some sort of check to help keep them in place.

> If we can't discard and recode, then that means the imgFrame didn't get
> SetDiscardable() called on it and we don't need to be careful about
> releasing the image data back to the OS

Thanks for the explanation! That should be documented in a comment. Beyond that, part of the reason that wasn't clear during the initial review is that you use a different test when you decide whether to call SetDiscardable initially. Why aren't they the same?

> This is probably a pretty good place to assert, since we should never return
> nullptr unless it's a single pixel optimized image, indexed image, or an OS
> discarded image.

I'm all for placing assertions anywhere you think they'd be useful.
Assignee

Comment 32

5 years ago
This is kinda horrible, but it's the most efficient way to work with SetData. The alternative is to allocate a VolatileBufferPtr, which seems like a waste.. (I do have a patch for that if that's preferred)
Attachment #8372658 - Attachment is obsolete: true
Attachment #8376878 - Flags: review?(mh+mozilla)
Attachment #8376878 - Flags: review?(jmuizelaar)
Assignee

Comment 33

5 years ago
Ok, this takes a different approach. When we have a discardable image, we use a weak pointer to hold onto the ImageContainer. Requested comments also added.

Haven't had a chance to measure the time taken by locking volatile buffers yet.
Attachment #8369086 - Attachment is obsolete: true
Attachment #8374635 - Attachment is obsolete: true
Attachment #8369086 - Flags: review?(roc)
Attachment #8374635 - Flags: review?(seth)
Attachment #8374635 - Flags: review?(jmuizelaar)
Attachment #8376915 - Flags: review?(seth)
Attachment #8376915 - Flags: review?(jmuizelaar)
Comment on attachment 8376878 [details] [diff] [review]
[1/2] Add gfxLockedImageSurface, v6

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

Not sure what i'm supposed to review here.
Attachment #8376878 - Flags: review?(mh+mozilla)
Comment on attachment 8376878 [details] [diff] [review]
[1/2] Add gfxLockedImageSurface, v6

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

::: gfx/thebes/gfxLockedImageSurface.h
@@ +11,5 @@
> +/*
> + * This is just a gfxImageSurface which will unlock the buffer on destruction
> + */
> +
> +class gfxLockedImageSurface : public gfxImageSurface

I think you can just turn this into a helper class. You shouldn't need to inherit from gfxImageSurface. Just have the users of this set SetData() on the surfaces they are using.
Attachment #8376878 - Flags: review?(jmuizelaar) → review-
Assignee

Comment 36

5 years ago
I was also thinking inheriting from gfxImageSurface is pointless now.
Attachment #8376878 - Attachment is obsolete: true
Attachment #8381152 - Flags: review?(jmuizelaar)
Assignee

Comment 37

5 years ago
Comment on attachment 8381152 [details] [diff] [review]
[1/2] Add gfxLockedImageSurface, v7

Requesting review from glandium for the VolatileBuffer api change.
Attachment #8381152 - Flags: review?(mh+mozilla)
Assignee

Comment 38

5 years ago
Adjusted patch due to changes in gfxLockedImageSurface.
Attachment #8376915 - Attachment is obsolete: true
Attachment #8376915 - Flags: review?(seth)
Attachment #8376915 - Flags: review?(jmuizelaar)
Attachment #8381154 - Flags: review?(seth)
Attachment #8381154 - Flags: review?(jmuizelaar)
Attachment #8381154 - Attachment is patch: true
Comment on attachment 8381154 [details] [diff] [review]
[2/2] Make imgFrame store images in VolatileBuffers, v11

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

Thanks for adding those comments. They make your intent much clearer.

::: image/src/imgFrame.h
@@ +195,5 @@
>  
>    /** Indicates how many readers currently have locked this frame */
>    int32_t mLockCount;
>  
> +  mozilla::RefPtr<mozilla::VolatileBuffer> mVBuf;

This seems like a pretty classic case for |mutable| but I don't think we need to worry too much about usage of const imgFrame references right now so I'm happy either way.
Attachment #8381154 - Flags: review?(seth) → review+
Comment on attachment 8381152 [details] [diff] [review]
[1/2] Add gfxLockedImageSurface, v7

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

It would be better to avoid exposing those guts. How about this:

::: gfx/thebes/gfxLockedImageSurface.cpp
@@ +14,5 @@
> +VolatileBufferRelease(void *buf)
> +{
> +    VolatileBuffer *vbuf = static_cast<VolatileBuffer*>(buf);
> +    vbuf->Unlock();
> +    vbuf->Release();

VolatileBufferPtr *vbuf = static_cast<VolatileBufferPtr*>(buf);
delete vbuf;

@@ +35,5 @@
> +
> +    void *buf;
> +    vbuf->AddRef();
> +    vbuf->Lock(&buf);
> +    img->SetData(&kVolatileBuffer, vbuf, VolatileBufferRelease);

VolatileBufferPtr *buf = new VolatileBufferPtr(vbuf);
img->SetData(&kVolatileBuffer, buf, VolatileBufferRelease);

(I wonder if it wouldn't be possible to use operator delete directly here instead of VolatileBufferRelease)
Attachment #8381152 - Flags: review?(mh+mozilla) → feedback-
Assignee

Comment 41

5 years ago
I combined the patches into one. gfxLockedImageSurface is now in imgFrame.cpp, and will probably be removed in the future with some other patches I'm working on.

The time taken to lock a volatile buffer generally appears to be around 5-15 microseconds on a relatively recent Mac Mini running OSX 10.9 with a 2.3ghz i7. Occasionally goes over 20 microseconds, but it's very rare. Also occasionally takes around 200 nanoseconds, which I assume is the case when we're already locked and are taking the fast path that just increments a counter.
Attachment #8381152 - Attachment is obsolete: true
Attachment #8381154 - Attachment is obsolete: true
Attachment #8381152 - Flags: review?(jmuizelaar)
Attachment #8381154 - Flags: review?(jmuizelaar)
Attachment #8384357 - Flags: review?(jmuizelaar)
Assignee

Comment 42

5 years ago
Just tested on a Flame device (1.2ghz dual core - MSM8610). Seems to take around 15-20 microseconds with the occasional jump to 40-50 microseconds. Fast path takes about 1 microsecond.
Assignee

Updated

5 years ago
Blocks: 980035
Assignee

Comment 43

5 years ago
Comment on attachment 8384357 [details] [diff] [review]
Make imgFrame store images in VolatileBuffers, v12

Seth, do you mind reviewing the locked image surface stuff that I've moved into RasterImage.cpp?
Attachment #8384357 - Flags: review?(jmuizelaar) → review?(seth)
Assignee

Updated

5 years ago
Blocks: 980037
Comment on attachment 8384357 [details] [diff] [review]
Make imgFrame store images in VolatileBuffers, v12

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

::: image/src/imgFrame.cpp
@@ +626,5 @@
> +  if (!mImageSurface) {
> +    if (mVBuf) {
> +      VolatileBufferPtr<uint8_t> ref(mVBuf);
> +      if (ref.WasBufferPurged())
> +        return NS_ERROR_FAILURE;

LockedImageSurface::CreateSurface does this same check. CreateSurface should either take a VolatileBufferPtr or this check should be omitted.
Attachment #8384357 - Flags: review?(seth) → review?(jmuizelaar)
Assignee

Comment 45

5 years ago
Comment on attachment 8384357 [details] [diff] [review]
Make imgFrame store images in VolatileBuffers, v12

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

::: image/src/imgFrame.cpp
@@ +626,5 @@
> +  if (!mImageSurface) {
> +    if (mVBuf) {
> +      VolatileBufferPtr<uint8_t> ref(mVBuf);
> +      if (ref.WasBufferPurged())
> +        return NS_ERROR_FAILURE;

The check in CreateSurface is only an assertion and wouldn't happen on normal builds. The CreateSurface caller needs to ensure that their image data is still available before wrapping it with a gfxImageSurface.
(In reply to Michael Wu [:mwu] from comment #45)
> Comment on attachment 8384357 [details] [diff] [review]
> Make imgFrame store images in VolatileBuffers, v12
> 
> Review of attachment 8384357 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: image/src/imgFrame.cpp
> @@ +626,5 @@
> > +  if (!mImageSurface) {
> > +    if (mVBuf) {
> > +      VolatileBufferPtr<uint8_t> ref(mVBuf);
> > +      if (ref.WasBufferPurged())
> > +        return NS_ERROR_FAILURE;
> 
> The check in CreateSurface is only an assertion and wouldn't happen on
> normal builds. The CreateSurface caller needs to ensure that their image
> data is still available before wrapping it with a gfxImageSurface.

Wouldn't it be better for CreateSurface to just take a VolatileBufferPtr then?
Assignee

Comment 47

5 years ago
(In reply to Jeff Muizelaar [:jrmuizel] from comment #46)
> (In reply to Michael Wu [:mwu] from comment #45)
> > Comment on attachment 8384357 [details] [diff] [review]
> > Make imgFrame store images in VolatileBuffers, v12
> > 
> > Review of attachment 8384357 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: image/src/imgFrame.cpp
> > @@ +626,5 @@
> > > +  if (!mImageSurface) {
> > > +    if (mVBuf) {
> > > +      VolatileBufferPtr<uint8_t> ref(mVBuf);
> > > +      if (ref.WasBufferPurged())
> > > +        return NS_ERROR_FAILURE;
> > 
> > The check in CreateSurface is only an assertion and wouldn't happen on
> > normal builds. The CreateSurface caller needs to ensure that their image
> > data is still available before wrapping it with a gfxImageSurface.
> 
> Wouldn't it be better for CreateSurface to just take a VolatileBufferPtr
> then?

VolatileBufferPtrs are normally allocated on stack. It's a bit unusual here that we're allocating one here on heap - basically no other code will need to do that.
Assignee

Updated

5 years ago
blocking-b2g: --- → 1.3T?
Assignee

Updated

5 years ago
Blocks: 980508
Attachment #8384357 - Flags: review?(jmuizelaar) → review+
Assignee

Comment 50

5 years ago
Thanks philor. Looks like I leaked a gfxImageSurface on a OSX specific path in ThebesSurface().
Assignee

Comment 51

5 years ago
Let's try again. I had to modify the gfxQuartzImageSurface path in imgFrame::ThebesSurface().

https://hg.mozilla.org/integration/b2g-inbound/rev/2429468e82dd
Could this cause bug 981082?
Assignee

Comment 53

5 years ago
(In reply to Jeff Muizelaar [:jrmuizel] from comment #52)
> Could this cause bug 981082?

Probably not - it's not done landing yet.
https://hg.mozilla.org/mozilla-central/rev/2429468e82dd
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Assignee

Comment 55

5 years ago
Looks like this regressed SVG-ASAP by about 10% or so. I think we can reduce the number of allocations to speed things back up.
blocking-b2g: 1.3T? → 1.3T+
Hi Ying Xu, heard that you will be doing uplifts to 1.3T branch. After you completed the uplift, can you please set status-b2g-v1.3T to fixed? please let us know if you have problems with it. thanks
Flags: needinfo?(ying.xu)

Comment 57

5 years ago
Hi,mwu

Could you please rebase your patch on branch v1.3t?
There are some conflicts for the current patch.
Flags: needinfo?(ying.xu) → needinfo?(mwu)
Assignee

Comment 58

5 years ago
I'll check this patch in myself. It depends on a few other patches in order to apply properly.
Flags: needinfo?(mwu)
Assignee

Comment 59

5 years ago
I uplifted these bugs to port this patch:

Bug 958758
Bug 890743

Requesting blocking on them.
blocking-b2g: 1.3T+ → 1.3T?
Assignee

Comment 60

5 years ago
er, I accidentally unset that blocking flag
Assignee

Updated

5 years ago
Depends on: 958758, 890743
blocking-b2g: 1.3T? → 1.3T+
Assignee

Updated

5 years ago
Blocks: 985017
Blocks: 989375
Depends on: 995880
Assignee

Updated

5 years ago
Blocks: 996226

Updated

5 years ago
Depends on: 998754
Depends on: 1044193
Depends on: 1380649
You need to log in before you can comment on or make changes to this bug.