The default bug view has changed. See this FAQ.

We have hundreds of copies of the UA string override on B2G

RESOLVED FIXED in mozilla25

Status

()

Core
Networking: HTTP
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: mccr8, Assigned: dao)

Tracking

Trunk
mozilla25
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink:P3])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

4 years ago
I'm not sure if this is the right component, and I'm not sure how big of a deal this is, but it looks like the user agent override is holding alive a few hundreds copies of the UA string 'Mozilla/5.0 (Android; Mobile; rv:18.1) Gecko/18.1 Firefox/18.1'.  It would be good if we could avoid this somehow.

Here is a list of paths keeping alive one particular copy of this string. The other ones look similar, except with a different URL in the final step.  Let me know if you have trouble understanding this.

via resource://gre/modules/UserAgentOverrides.jsm :
0x4481a5a0 [FakeBackstagePass <no private>]
    --[UserAgentOverrides]-> 0x48e7dd60 [Object <no private>]
    --[getOverrideForURI]-> 0x48ebdd60 [Function uao_getOverrideForURI]
    --[fun_callscope]-> 0x44820820 [Call <no private>]
    --[gOverrides]-> 0x4a36f3a0 [Object <no private>]
    --[deadline.com]-> 0x4a343c30 [string Mozilla/5.0 (Android; Mobile; rv:18.1) Gecko/18.1 Firefox/18.1]

via XPCWrappedNative::mFlatJSObject :
0x48e46040 [ChromeWindow 48c77160]
    --[UserAgentOverrides]-> 0x48e4b5e0 [Proxy <no private>]
    --[private]-> 0x48e7dd60 [Object <no private>]
    --[getOverrideForURI]-> 0x48ebdd60 [Function uao_getOverrideForURI]
    --[fun_callscope]-> 0x44820820 [Call <no private>]
    --[gOverrides]-> 0x4a36f3a0 [Object <no private>]
    --[deadline.com]-> 0x4a343c30 [string Mozilla/5.0 (Android; Mobile; rv:18.1) Gecko/18.1 Firefox/18.1]

via nsXPCWrappedJS[nsIObserver,0x4a040400:0x47de6100].mJSObj :
0x48ebdcc0 [Function buildOverrides]
    --[fun_callscope]-> 0x44820820 [Call <no private>]
    --[gOverrides]-> 0x4a36f3a0 [Object <no private>]
    --[deadline.com]-> 0x4a343c30 [string Mozilla/5.0 (Android; Mobile; rv:18.1) Gecko/18.1 Firefox/18.1]

via nsXPCWrappedJS[nsIObserver,0x4a040480:0x47de6140].mJSObj :
0x48ebdce0 [Function HTTP_on_modify_request]
    --[fun_callscope]-> 0x44820820 [Call <no private>]
    --[gOverrides]-> 0x4a36f3a0 [Object <no private>]
    --[deadline.com]-> 0x4a343c30 [string Mozilla/5.0 (Android; Mobile; rv:18.1) Gecko/18.1 Firefox/18.1]
Whiteboard: [MemShrink] → [MemShrink:P3]
(Assignee)

Updated

4 years ago
Assignee: nobody → dao
(Assignee)

Comment 1

4 years ago
Created attachment 780266 [details] [diff] [review]
patch

With http://mxr.mozilla.org/gaia/source/build/ua-override-prefs.js?raw=1 applied on desktop, about:memory gave me the following numbers.

before:

73,504 B (00.04%) -- compartment([System Principal], resource://gre/modules/UserAgentOverrides.jsm)
├──49,488 B (00.03%) -- gc-heap
│  ├──20,360 B (00.01%) -- shapes
│  │  ├──11,320 B (00.01%) ── tree/global-parented
│  │  └───9,040 B (00.01%) ── dict
│  ├──16,448 B (00.01%) ── objects/function
│  └──12,680 B (00.01%) ── sundries
├──14,800 B (00.01%) ── other-sundries
└───9,216 B (00.01%) ── shapes-extra/compartment-tables

after:

63,584 B (00.04%) -- compartment([System Principal], resource://gre/modules/UserAgentOverrides.jsm)
├──43,584 B (00.03%) -- gc-heap
│  ├──16,512 B (00.01%) ── objects/function
│  ├──15,632 B (00.01%) ── sundries
│  └──11,440 B (00.01%) ── shapes/tree/global-parented
├──10,784 B (00.01%) ── other-sundries
└───9,216 B (00.01%) ── shapes-extra/compartment-tables

Is this the difference you'd expect?
Attachment #780266 - Flags: feedback?(continuation)
(Assignee)

Comment 2

4 years ago
Created attachment 780269 [details] [diff] [review]
patch v2

cleaned it up a bit. new "after" measurement:

63,320 B (00.03%) -- compartment([System Principal], resource://gre/modules/UserAgentOverrides.jsm)
├──43,416 B (00.02%) -- gc-heap
│  ├──16,512 B (00.01%) ── objects/function
│  ├──15,504 B (00.01%) ── sundries
│  └──11,400 B (00.01%) ── shapes/tree/global-parented
├──10,688 B (00.01%) ── other-sundries
└───9,216 B (00.01%) ── shapes-extra/compartment-tables
Attachment #780266 - Attachment is obsolete: true
Attachment #780266 - Flags: feedback?(continuation)
Attachment #780269 - Flags: feedback?(continuation)
(Assignee)

Comment 3

4 years ago
My patches apply on top of bug 896382's patch, which I just landed on inbound.
Depends on: 896382
(Reporter)

Comment 4

4 years ago
From the memory report there, it looks like you are saving 10k in shapes/dict, which I think would be the result of switching from an object to a map.  So that's good, but it doesn't involve the strings per se.  I don't actually see strings in that bit of the report at all.

If you get me a GC heap dump (using this https://wiki.mozilla.org/Performance:Leak_Tools#Cycle_collector_heap_dump ) then I can see if there are less strings floating around, or you can look at it yourself using this script: https://github.com/amccreight/heapgraph/blob/master/gc/stringy.py
(Assignee)

Comment 5

4 years ago
(In reply to Andrew McCreight [:mccr8] from comment #4)
> If you get me a GC heap dump (using this
> https://wiki.mozilla.org/Performance:Leak_Tools#Cycle_collector_heap_dump )

I was unable to locate the log file. No idea where it was written to. It's not in the path Firefox was running from.
(Reporter)

Comment 6

4 years ago
It should print out the place it dumps it to in the Browser Console.  On OSX, I think it ends up in ~/Library/Caches/TemporaryItems.  On Linux it is in /tmp I think
(Assignee)

Comment 7

4 years ago
(In reply to Andrew McCreight [:mccr8] from comment #6)
> It should print out the place it dumps it to in the Browser Console.

I just got "undefined" back.

> On Linux it is in /tmp I think

Ok, got it.
(Assignee)

Comment 8

4 years ago
Created attachment 780622 [details]
gc-edges.3608.log.bz2
(Reporter)

Comment 9

4 years ago
Yeah, I only see 19 strings in there that starting with "Mozilla", so that looks like an improvement, assuming what I was seeing before wasn't an artifact of the leak, which seems totally reasonable.  Thanks.
(Assignee)

Comment 10

4 years ago
Created attachment 780671 [details] [diff] [review]
patch v3

more cleanup
Attachment #780269 - Attachment is obsolete: true
Attachment #780269 - Flags: feedback?(continuation)
Attachment #780671 - Flags: review?(hurley)
(Assignee)

Comment 11

4 years ago
(In reply to Andrew McCreight [:mccr8] from comment #9)
> Yeah, I only see 19 strings in there that starting with "Mozilla", so that
> looks like an improvement, assuming what I was seeing before wasn't an
> artifact of the leak, which seems totally reasonable.  Thanks.

I looked at the log myself and compared it with a log from an unpatched build. You definitely didn't see a leak and the patch brings UserAgentOverrides.jsm on b2g down to three strings (one for DEFAULT_UA, two in the gOverrides map).
Comment on attachment 780671 [details] [diff] [review]
patch v3

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

This all seems reasonable to me, but given my lack of status as a necko peer, I'm going to hand off to Jason for the official review.

Jason - this touches code from the other user agent override patch I looked at earlier this week, so don't feel like you have to go over this with a fine-toothed comb or anything. I'm just following protocol :)
Attachment #780671 - Flags: review?(jduell.mcbugs)
Attachment #780671 - Flags: review?(hurley)
Attachment #780671 - Flags: feedback+
Comment on attachment 780671 [details] [diff] [review]
patch v3

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

Seems fine especially since nick thinks so :)
Attachment #780671 - Flags: review?(jduell.mcbugs) → review+
(Assignee)

Comment 14

4 years ago
https://hg.mozilla.org/integration/fx-team/rev/37567e5bbe9f
(Assignee)

Comment 15

4 years ago
Apparently this got merged yesterday.

https://hg.mozilla.org/mozilla-central/rev/37567e5bbe9f
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.