Closed Bug 916060 Opened 8 years ago Closed 7 years ago

Each homescreen icon cost ~60K of memory

Categories

(Firefox OS Graveyard :: Gaia::Homescreen, defect, P1)

All
Gonk (Firefox OS)
defect

Tracking

(Not tracked)

RESOLVED WONTFIX
1.4 S2 (28feb)

People

(Reporter: vingtetun, Assigned: jhylands)

References

Details

(Keywords: perf, Whiteboard: [c=memory p=3 s= u=tarako] [MemShrink:P2][~60k * icon number])

Trying to activate everyrything.me smart collection we noticed an increase in memory consumption of the homescreen. This extra memory consumption is proportional to the number of icons on the screen and it happens even if everything.me code is lazy loaded.

Some manual test to add more icons reveal that it happens for all icons: application icons, bookmarks icons, smart collection icons.

What it means is that every time the user install a new app it will end up with a homescreen that consume more memory, resulting less memory available for the others apps as well as a homescreen that will be killed more often.

This stuff blocks e.me integration since the bug is much more obvious with smart collections activated on the main home page...
Hey Mike, could figure out who's the best assignee for this issue ? It's a high priority.
Flags: needinfo?(mlee)
Keywords: perf
(In reply to David Scravaglieri [:scravag] from comment #1)
> Hey Mike, could figure out who's the best assignee for this issue ? It's a
> high priority.

This is a memory issue, not a perf issue. Should have MemShrink in the whiteboard.
Keywords: perf
Whiteboard: [MemShrink]
Vivien,
Are Smart Collections landing in 1.2 (koi) or 1.3?

Jason,
You're correct this is a memory issue to be handled by the MemShrink team, but it's still a perf issue and we (fxos-perf) need to track this so readding the "perf" keyword.

+Kyle Huey (MemShrink for B2G)
Flags: needinfo?(mlee) → needinfo?(21)
Keywords: perf
Whiteboard: [MemShrink] → [MemShrink] [c=memory p= s= u=]
Is it possible to get about:memory dumps with N and N+1 icons?
(In reply to Mike Lee [:mlee] from comment #3)
> Vivien,
> Are Smart Collections landing in 1.2 (koi) or 1.3?
> 
> Jason,
> You're correct this is a memory issue to be handled by the MemShrink team,
> but it's still a perf issue and we (fxos-perf) need to track this so
> readding the "perf" keyword.

The standard use of this keyword does not follow this precedent - I checked with other QAs on other products. Please see the definition of the keyword that states perf is supposed to be used for:

A bug that affects speed or responsiveness. (For memory use issues, use "footprint" or "mlk" instead.)

For memory issues, MemShrink is the whiteboard to use. footprint was a keyword defined for the usage, but apparently isn't advocated in bugzilla. I do not want to confuse the MemShrink team from introducing multiple tracking processes and suggesting that we clearly separate responsiveness issues from memory issues. Please address your query to analyze MemShrink in the whiteboard and not globally analyze the perf keyword solely - this is going to result in polluting the perf keyword otherwise and will confuse my team on the right flags to use.
Keywords: perf
(In reply to Mike Lee [:mlee] from comment #3)
> Vivien,
> Are Smart Collections landing in 1.2 (koi) or 1.3?
>

1.2
 
I will provide the about:memory asked by Kyle.
Flags: needinfo?(21)
Seems like 100K is not realistic.
Summary: Each homescreen icon cost ~100K of memory → Each homescreen icon cost ~60K of memory
Whiteboard: [MemShrink] [c=memory p= s= u=] → [MemShrink:P2] [c=memory p= s= u=]
Not so easy to reproduce with my instrumenting tools but this stack might be a start:

Bytes Used	Count		Symbol Name
 243.40 KB      86.1%	384	 	malloc
 112.00 KB      39.6%	7	 	 mozilla::gfx::SourceSurfaceCG::InitFromData(unsigned char*, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, int, mozilla::gfx::SurfaceFormat)
 112.00 KB      39.6%	7	 	  mozilla::gfx::DrawTargetCG::CreateSourceSurfaceFromData(unsigned char*, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, int, mozilla::gfx::SurfaceFormat) const
 112.00 KB      39.6%	7	 	   gfxPlatform::GetSourceSurfaceForSurface(mozilla::gfx::DrawTarget*, gfxASurface*)
 112.00 KB      39.6%	7	 	    gfxPattern::GetPattern(mozilla::gfx::DrawTarget*, mozilla::gfx::Matrix*)
 112.00 KB      39.6%	7	 	     gfxContext::FillAzure(float)
 112.00 KB      39.6%	7	 	      gfxContext::Fill()
 112.00 KB      39.6%	7	 	       gfxSurfaceDrawable::Draw(gfxContext*, gfxRect const&, bool, gfxPattern::GraphicsFilter const&, gfxMatrix const&)
 112.00 KB      39.6%	7	 	        gfxUtils::DrawPixelSnapped(gfxContext*, gfxDrawable*, gfxMatrix const&, gfxRect const&, gfxRect const&, gfxRect const&, gfxRect const&, gfxImageFormat, gfxPattern::GraphicsFilter, unsigned int)
 112.00 KB      39.6%	7	 	         imgFrame::Draw(gfxContext*, gfxPattern::GraphicsFilter, gfxMatrix const&, gfxRect const&, nsIntMargin const&, nsIntRect const&, unsigned int)
 112.00 KB      39.6%	7	 	          mozilla::image::RasterImage::DrawWithPreDownscaleIfNeeded(imgFrame*, gfxContext*, gfxPattern::GraphicsFilter, gfxMatrix const&, gfxRect const&, nsIntRect const&, unsigned int)
 112.00 KB      39.6%	7	 	           mozilla::image::RasterImage::Draw(gfxContext*, gfxPattern::GraphicsFilter, gfxMatrix const&, gfxRect const&, nsIntRect const&, nsIntSize const&, mozilla::SVGImageContext const*, unsigned int, unsigned int)
 112.00 KB      39.6%	7	 	            DrawImageInternal(nsRenderingContext*, imgIContainer*, gfxPattern::GraphicsFilter, nsRect const&, nsRect const&, nsPoint const&, nsRect const&, nsIntSize const&, mozilla::SVGImageContext const*, unsigned int)
 112.00 KB      39.6%	7	 	             nsLayoutUtils::DrawSingleImage(nsRenderingContext*, imgIContainer*, gfxPattern::GraphicsFilter, nsRect const&, nsRect const&, mozilla::SVGImageContext const*, unsigned int, nsRect const*)
 112.00 KB      39.6%	7	 	              nsImageFrame::PaintImage(nsRenderingContext&, nsPoint, nsRect const&, imgIContainer*, unsigned int)
 112.00 KB      39.6%	7	 	               nsDisplayImage::Paint(nsDisplayListBuilder*, nsRenderingContext*)
 112.00 KB      39.6%	7	 	                mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*)
 112.00 KB      39.6%	7	 	                 mozilla::layers::BasicThebesLayer::PaintThebes(gfxContext*, mozilla::layers::Layer*, void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*, mozilla::layers::ReadbackProcessor*)
 112.00 KB      39.6%	7	 	                  mozilla::layers::BasicLayerManager::PaintSelfOrChildren(mozilla::layers::PaintLayerContext&, gfxContext*)
 112.00 KB      39.6%	7	 	                   mozilla::layers::BasicLayerManager::PaintLayer(gfxContext*, mozilla::layers::Layer*, void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*, mozilla::layers::ReadbackProcessor*)
 112.00 KB      39.6%	7	 	                    mozilla::layers::BasicLayerManager::PaintSelfOrChildren(mozilla::layers::PaintLayerContext&, gfxContext*)
Can we get some gfx help here?
Flags: needinfo?(milan)
Flags: needinfo?(bugs)
I think Benoit may recall where it's all going.
Flags: needinfo?(milan)
Flags: needinfo?(bugs)
Flags: needinfo?(bjacob)
I seem to remember from a conversation with Vivien a month ago (pardon me if I misremember!) that each Everything.me icon consisted of no less than 3 (three) 2d canvases. Is that correct, Vivien? If yes, then we can only be thankful/hopeful that they don't consume more than 60k! The fix really is not using 2d canvases (let alone 3 of them) for each icon. There are many basic reasons why it would be very hard or impossible to implement 2d canvases in a way that doesn't make them use more memory than images.
Flags: needinfo?(bjacob)
Flags: needinfo?(21)
Flags: needinfo?(ran)
Benoit, this is not an Evme icon issure.
When we researched it we realized this is relevant to all grid icons including preinstalled apps, downloaded apps and even bookmarks (as well as the new Collection icon). We've also noticed every icon can exceed 60k substantially.
Flags: needinfo?(ran)
(In reply to Benoit Jacob [:bjacob] from comment #11)
> I seem to remember from a conversation with Vivien a month ago (pardon me if
> I misremember!) that each Everything.me icon consisted of no less than 3
> (three) 2d canvases. Is that correct, Vivien? If yes, then we can only be
> thankful/hopeful that they don't consume more than 60k! The fix really is
> not using 2d canvases (let alone 3 of them) for each icon. There are many
> basic reasons why it would be very hard or impossible to implement 2d
> canvases in a way that doesn't make them use more memory than images.

Yeah e.me icons were canvases but we replaced those with images for some performances reasons while scrolling.

Also for this particular bug this happens or all homescreen icons. My current assumption is that it is related to the homescreen code, and this is not a gfx bug. The size of each icon is the sum of some Js object + dom nodes + images that end up beeing heavy if you consider that it could be a lot of those.
Flags: needinfo?(21)
(In reply to Jason Smith [:jsmith] from comment #5)
> (In reply to Mike Lee [:mlee] from comment #3)
> > Vivien,
> > Are Smart Collections landing in 1.2 (koi) or 1.3?
> > 
> > Jason,
> > You're correct this is a memory issue to be handled by the MemShrink team,
> > but it's still a perf issue and we (fxos-perf) need to track this so
> > readding the "perf" keyword.
> 
> The standard use of this keyword does not follow this precedent - I checked
> with other QAs on other products. Please see the definition of the keyword
> that states perf is supposed to be used for:
> 
> A bug that affects speed or responsiveness. (For memory use issues, use
> "footprint" or "mlk" instead.)
> 
> For memory issues, MemShrink is the whiteboard to use. footprint was a
> keyword defined for the usage, but apparently isn't advocated in bugzilla. I
> do not want to confuse the MemShrink team from introducing multiple tracking
> processes and suggesting that we clearly separate responsiveness issues from
> memory issues. Please address your query to analyze MemShrink in the
> whiteboard and not globally analyze the perf keyword solely - this is going
> to result in polluting the perf keyword otherwise and will confuse my team
> on the right flags to use.

Talked this over with hub in IRC. We're going to go with both the whiteboard & keyword tracking-wise.
Keywords: perf
No longer blocks: 1.3-e.me
Removed block from v1.2 evme changes - there's no real correlation.
QA Contact: dhuseby
Whiteboard: [MemShrink:P2] [c=memory p= s= u=] → [MemShrink:P2] [c=memory p=1 s= u=1.3]
Assignee: nobody → dhuseby
QA Contact: dhuseby
Priority: -- → P1
Whiteboard: [MemShrink:P2] [c=memory p=1 s= u=1.3] → [MemShrink:P2] [c=memory p=2 s= u=1.3]
See Also: → 918325
Whiteboard: [MemShrink:P2] [c=memory p=2 s= u=1.3] → [MemShrink:P2] [c=memory p=2 s= u=]
Going to steal this as Dave is out on PTO.
Assignee: dhuseby → bkelly
Status: NEW → ASSIGNED
Whiteboard: [MemShrink:P2] [c=memory p=2 s= u=] → [MemShrink:P2] [c=memory p=2 s= u=][tarako]
Whiteboard: [MemShrink:P2] [c=memory p=2 s= u=][tarako] → [c=memory p=2 s= u=tarako] [MemShrink:P2][tarako]
Whiteboard: [c=memory p=2 s= u=tarako] [MemShrink:P2][tarako] → [c=memory p=2 s= u=tarako] [MemShrink:P2][tarako][~60k * icon number]
Whiteboard: [c=memory p=2 s= u=tarako] [MemShrink:P2][tarako][~60k * icon number] → [c=memory p=3 s= u=tarako] [MemShrink:P2][tarako][~60k * icon number]
Assignee: bkelly → jhylands
triage: 1.3T? to keep in tarako radar. it is believed that this bug is no longer needed as homescreen icons will get freed if memory is needed. can you briefly describe what the fix is? thanks
blocking-b2g: --- → 1.3T?
Flags: needinfo?(jhylands)
Whiteboard: [c=memory p=3 s= u=tarako] [MemShrink:P2][tarako][~60k * icon number] → [c=memory p=3 s= u=tarako] [MemShrink:P2][~60k * icon number]
I have no idea what the fix is at this point - I haven't even looked at this bug yet.
Flags: needinfo?(jhylands)
Joe, can you provide a link to the bug that fixed "it is believed that this bug is no longer needed as homescreen icons will get freed if memory is needed"?
Flags: needinfo?(jcheng)
Jon, we talked about this at the tarako triage.  If the 60k is coming from image data, then this may not be a problem because it will get discarded during memory pressure.  If the 60k is coming from something else, though, then it sounds like it still may be a problem.

So it does sound like this needs more investigation.  Hope that helps clarify a bit.  Sorry for the confusion.
Flags: needinfo?(jcheng)
After doing a bunch of analysis, I see about 44KB per icon when I add web shortcuts to the homescreen.

The breakdown is:

Total			44
js-non-window		2
  unused-gc-things	2
heap unclassified	2
Heap-overhead/waste	17
windows/objects/top	10
  js-compartment	2
  layout-frames		1
  dom-element nodes	3
images			4

These numbers are taken from the results of get_about_memory.py, averaged over 8 new icons added. 

Results of course may vary based on what gets added, but clearly most of the memory is not from image data, but most of it seems to be of the type that will go away with memory pressure.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
do you mean to mark this as resolved won't fix? thanks
Flags: needinfo?(jhylands)
You are of course correct.
Flags: needinfo?(jhylands)
Resolution: FIXED → WONTFIX
Clearing the nom as this is wontfix.
blocking-b2g: 1.3T? → ---
Target Milestone: --- → 1.4 S2 (28feb)
You need to log in before you can comment on or make changes to this bug.