Closed Bug 1442067 Opened 6 years ago Closed 6 years ago

Several megabytes of stack related strings in about:newtab

Categories

(Firefox :: Messaging System, defect, P2)

defect

Tracking

()

RESOLVED FIXED
Firefox 61
Iteration:
61.4 - May 7
Tracking Status
firefox60 --- wontfix
firefox61 --- fixed

People

(Reporter: erahm, Assigned: andreio)

References

(Blocks 1 open bug)

Details

(Whiteboard: [MemShrink:P2])

Attachments

(1 file, 1 obsolete file)

In a recent memory report I noticed ~2.3MB of strings which seem to be related to a stack trace (perhaps exception related). It appears activity streams is doing something pathologically weird here, the worst offender had 6,686 copies.

> │  ├───24,599,224 B (06.66%) -- top(about:newtab, id=6442451077)
> │  │   ├──15,193,824 B (04.11%) -- js-zone(0x7ff832532000)
> │  │   │  ├──11,811,592 B (03.20%) -- strings
> ... snip ...
> │  │   │  │  ├─────158,064 B (00.04%) ── string(length=11, copies=6586, "/n    in di")/gc-heap/latin1
> │  │   │  │  ├─────132,192 B (00.04%) ── string(length=23, copies=4131, " (created by Base__Base")/gc-heap/latin1
> │  │   │  │  ├─────128,808 B (00.03%) ── string(length=29, copies=5367, " (created by Base_BaseContent")/gc-heap/latin1
> │  │   │  │  ├─────128,808 B (00.03%) ── string(length=30, copies=5367, " (created by Base_BaseContent)")/gc-heap/latin1
> │  │   │  │  ├──────99,144 B (00.03%) ── string(length=24, copies=4131, " (created by Base__Base)")/gc-heap/latin1
> │  │   │  │  ├──────79,808 B (00.02%) ── string(length=21, copies=2494, "/n    in ErrorBoundar")/gc-heap/latin1
> │  │   │  │  ├──────74,112 B (00.02%) ── string(length=32, copies=3088, " (created by _CollapsibleSection")/gc-heap/latin1
> │  │   │  │  ├──────74,112 B (00.02%) ── string(length=33, copies=3088, " (created by _CollapsibleSection)")/gc-heap/latin1
> │  │   │  │  ├──────65,544 B (00.02%) ── string(length=41, copies=2731, "/n    in div (created by Base_BaseContent")/gc-heap/latin1
> │  │   │  │  ├──────51,456 B (00.01%) ── string(length=22, copies=1608, " (created by _TopSites")/gc-heap/latin1
> │  │   │  │  ├──────51,456 B (00.01%) ── string(length=23, copies=1608, " (created by _TopSites)")/gc-heap/latin1
> │  │   │  │  ├──────46,528 B (00.01%) ── string(length=20, copies=1454, " (created by Section")/gc-heap/latin1
> │  │   │  │  ├──────46,528 B (00.01%) ── string(length=21, copies=1454, " (created by Section)")/gc-heap/latin1
> │  │   │  │  ├──────44,064 B (00.01%) ── string(length=18, copies=1377, "/n    in Base__Bas")/gc-heap/latin1
> │  │   │  │  ├──────44,064 B (00.01%) ── string(length=20, copies=1377, "/n    in IntlProvide")/gc-heap/latin1
> │  │   │  │  ├──────41,088 B (00.01%) ── string(length=20, copies=1284, " (created by TopSite")/gc-heap/latin1
> │  │   │  │  ├──────41,088 B (00.01%) ── string(length=21, copies=1284, " (created by TopSite)")/gc-heap/latin1
> │  │   │  │  ├──────34,704 B (00.01%) ── string(length=45, copies=1446, " (created by PreferencesPane__PreferencesPane")/gc-heap/latin1
> │  │   │  │  ├──────34,704 B (00.01%) ── string(length=46, copies=1446, " (created by PreferencesPane__PreferencesPane)")/gc-heap/latin1
> │  │   │  │  ├──────33,048 B (00.01%) ── string(length=24, copies=1377, "/n    in Base_BaseConten")/gc-heap/latin1
> │  │   │  │  ├──────33,048 B (00.01%) ── string(length=27, copies=1377, "/n    in Connect(Base__Base")/gc-heap/latin1
> │  │   │  │  ├──────33,048 B (00.01%) ── string(length=32, copies=1377, " (created by Connect(Base__Base)")/gc-heap/latin1
> │  │   │  │  ├──────33,048 B (00.01%) ── string(length=33, copies=1377, " (created by Connect(Base__Base))")/gc-heap/latin1
> │  │   │  │  ├──────33,048 B (00.01%) ── string(length=44, copies=1377, "/n    in IntlProvider (created by Base__Base")/gc-heap/latin1
> │  │   │  │  ├──────33,048 B (00.01%) ── string(length=45, copies=1377, "/n    in ErrorBoundary (created by Base__Base")/gc-heap/latin1
> │  │   │  │  ├──────33,048 B (00.01%) ── string(length=48, copies=1377, "/n    in Base_BaseContent (created by Base__Base")/gc-heap/latin1
> │  │   │  │  ├──────33,048 B (00.01%) ── string(length=51, copies=1377, "/n    in Base__Base (created by Connect(Base__Base)")/gc-heap/latin1
> │  │   │  │  ├──────33,048 B (00.01%) ── string(length=9, copies=1377, "/n    in ")/gc-heap/latin1
> │  │   │  │  ├──────32,832 B (00.01%) ── string(length=24, copies=1368, " (created by ContextMenu")/gc-heap/latin1
> │  │   │  │  ├──────32,832 B (00.01%) ── string(length=25, copies=1368, " (created by ContextMenu)")/gc-heap/latin1
> │  │   │  │  ├──────29,928 B (00.01%) ── string(length=10, copies=1247, "/n    in u")/gc-heap/latin1
> │  │   │  │  ├──────29,688 B (00.01%) ── string(length=15, copies=1237, "/n    in sectio")/gc-heap/latin1
> │  │   │  │  ├──────28,992 B (00.01%) ── string(length=10, copies=1208, "/n    in l")/gc-heap/latin1
> │  │   │  │  ├──────27,776 B (00.01%) ── string(length=22, copies=868, " (created by _Sections")/gc-heap/latin1
> │  │   │  │  ├──────27,776 B (00.01%) ── string(length=23, copies=868, " (created by _Sections)")/gc-heap/latin1
> │  │   │  │  ├──────27,456 B (00.01%) ── string(length=24, copies=1144, " (created by TopSiteLink")/gc-heap/latin1
> │  │   │  │  ├──────27,456 B (00.01%) ── string(length=25, copies=1144, " (created by TopSiteLink)")/gc-heap/latin1
> │  │   │  │  ├──────26,944 B (00.01%) ── string(length=22, copies=842, " (created by Card_Card")/gc-heap/latin1
> │  │   │  │  ├──────26,944 B (00.01%) ── string(length=23, copies=842, " (created by Card_Card)")/gc-heap/latin1
> │  │   │  │  ├──────26,928 B (00.01%) ── string(length=57, copies=1122, "/n    in div (created by PreferencesPane__PreferencesPane")/gc-heap/latin1
> │  │   │  │  ├──────25,200 B (00.01%) ── string(length=44, copies=1050, "/n    in div (created by _CollapsibleSection")/gc-heap/latin1
> │  │   │  │  ├──────25,032 B (00.01%) ── string(length=12, copies=1043, "/n    in mai")/gc-heap/latin1
> │  │   │  │  ├──────25,032 B (00.01%) ── string(length=42, copies=1043, "/n    in main (created by Base_BaseContent")/gc-heap/latin1
> │  │   │  │  ├──────23,688 B (00.01%) ── string(length=25, copies=987, " (created by _TopSiteList")/gc-heap/latin1
> │  │   │  │  ├──────23,688 B (00.01%) ── string(length=26, copies=987, " (created by _TopSiteList)")/gc-heap/latin1
> │  │   │  │  ├──────23,640 B (00.01%) ── string(length=26, copies=985, "/n    in ComponentPerfTime")/gc-heap/latin1
> │  │   │  │  ├──────23,640 B (00.01%) ── string(length=27, copies=985, "/n    in _CollapsibleSectio")/gc-heap/latin1
> │  │   │  │  ├──────23,640 B (00.01%) ── string(length=39, copies=985, "/n    in InjectIntl(_CollapsibleSection")/gc-heap/latin1
> │  │   │  │  ├──────23,640 B (00.01%) ── string(length=44, copies=985, " (created by InjectIntl(_CollapsibleSection)")/gc-heap/latin1
> │  │   │  │  ├──────23,640 B (00.01%) ── string(length=45, copies=985, " (created by InjectIntl(_CollapsibleSection))")/gc-heap/latin1
> │  │   │  │  ├──────23,640 B (00.01%) ── string(length=48, copies=985, "/n    in section (created by _CollapsibleSection")/gc-heap/latin1
> │  │   │  │  ├──────23,640 B (00.01%) ── string(length=72, copies=985, "/n    in _CollapsibleSection (created by InjectIntl(_CollapsibleSection)")/gc-heap/latin1
> │  │   │  │  ├──────23,376 B (00.01%) ── string(length=9, copies=974, "classname")/gc-heap/latin1
> │  │   │  │  ├──────21,432 B (00.01%) ── string(length=12, copies=893, "/n    in spa")/gc-heap/latin1
> │  │   │  │  ├──────19,608 B (00.01%) ── string(length=54, copies=817, "/n    in ErrorBoundary (created by _CollapsibleSection")/gc-heap/latin1
> │  │   │  │  ├──────18,432 B (00.00%) ── string(length=28, copies=768, " (created by ContextMenuItem")/gc-heap/latin1
> │  │   │  │  ├──────18,432 B (00.00%) ── string(length=29, copies=768, " (created by ContextMenuItem)")/gc-heap/latin1
> │  │   │  │  └──────17,760 B (00.00%) ── string(length=17, copies=555, "/n    in _TopSite")/gc-heap/latin1

Note the '/n' is just an escaped newline.
Seems like it probably came from react-dev:
https://searchfox.org/mozilla-central/source/browser/extensions/activity-stream/vendor/react-dev.js#1234

Bug 1428108 did land recently turning on debug mode by default for local builds. Does flipping browser.newtabpage.activity-stream.debug to false avoid some of that string usage?
Depends on: 1428108
(In reply to Ed Lee :Mardak from comment #1)
> Seems like it probably came from react-dev:
> https://searchfox.org/mozilla-central/source/browser/extensions/activity-
> stream/vendor/react-dev.js#1234
> 
> Bug 1428108 did land recently turning on debug mode by default for local
> builds. Does flipping browser.newtabpage.activity-stream.debug to false
> avoid some of that string usage?

Yeah setting that pref clears things up.
Maybe we should put this behind a mozconfig variable?
Priority: -- → P3
Is this still relevant?  does that pref still exist?
Flags: needinfo?(tspurway)
Priority: P3 → --
We are probably going to leave this as a pref, but could change it to false to default.  Would this work, :jesup?
Flags: needinfo?(tspurway) → needinfo?(rjesup)
Basically reverting bug 1428108 https://hg.mozilla.org/mozilla-central/rev/e7d8e299023a

dmose, why do we want this on for all devs instead of a subset, e.g., activity stream devs?
Flags: needinfo?(dmose)
(In reply to Tim Spurway [:tspurway] from comment #5)
> We are probably going to leave this as a pref, but could change it to false
> to default.  Would this work, :jesup?

false is fine...  anyone who wants it on should have that in their .mozconfig or profile's prefs.js
Flags: needinfo?(rjesup)
Having it on activity-stream devs only would be fine.  A reasonable forward could be to set the default based on the (existing) .mozconfig / AppConstants.jsm DEBUG_JS_MODULES variable.
Flags: needinfo?(dmose)
Whiteboard: [MemShrink] → [MemShrink:P2]
Iteration: --- → 61.3 - Apr 23
Priority: -- → P2
Assignee: nobody → andrei.br92
Reverted bug 1428108 and added a function to read an environment variable that can force Activity Stream debug. This way other people won't accidentally trigger debug mode unless they toggle the pref or specifically expose the variable.
Attachment #8969632 - Flags: review?(edilee)
Attachment #8969633 - Flags: review?(edilee)
Comment on attachment 8969633 [details]
Bug 1442067 - Read environment variable that forces Activity Stream debug mode.

https://reviewboard.mozilla.org/r/238430/#review244396

What is this change for? I don't think we want to be adding new environment variables
Attachment #8969633 - Flags: review?(edilee) → review-
Comment on attachment 8969632 [details]
Bug 1442067 - Revert 1428108 set browser.newtabpage.activity-stream.debug by default in local builds.

https://reviewboard.mozilla.org/r/238428/#review244402
Attachment #8969632 - Flags: review?(edilee) → review+
If we're expecting someone to set something in .mozconfig or environment, then might as well just have the person use ./mach run --setpref browser.newtabpage.activity-stream.debug=true

Or if not wanting to do that each time or creating an alias, set it once with the [runprefs] ./mach settings
Attachment #8969633 - Attachment is obsolete: true
Keywords: checkin-needed
Iteration: 61.3 - Apr 23 → 61.4 - May 7
Pushed by ebalazs@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/804f8b73b13a
Revert 1428108 set browser.newtabpage.activity-stream.debug by default in local builds. r=Mardak
Keywords: checkin-needed
This made it to m-c a while back.. not sure why it wasn't automatically resolved?

https://hg.mozilla.org/mozilla-central/rev/804f8b73b13a
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
(In reply to Ed Lee :Mardak from comment #16)
> not sure why it wasn't automatically resolved?
Bug 1458490
Component: Activity Streams: Newtab → Messaging System
You need to log in before you can comment on or make changes to this bug.