about:memory's /proc/smaps breakdown missing on my phone, again

RESOLVED FIXED in mozilla10

Status

()

Toolkit
about:memory
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Justin Lebar (not reading bugmail), Unassigned)

Tracking

Trunk
mozilla10
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [inbound])

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
My phone is missing the /proc/smaps breakdown, again (bug 685870 was the first time).

I suspect, like the last time, we're being too strict in what we expect out of /proc/smaps.  I'm not sure what's going on this time, though.
(Reporter)

Comment 1

7 years ago
Okay, so this regression is due to a change in how we handle shared libraries.

Glandium, I see a bunch of entries in smaps like

  72000000-72074000 r-xs 0 00:04 982065     /dev/ashmem/libxul.so (deleted)

Do you know why it says "(deleted)"?  Smaps claims the mapping is indeed taking up resident space...
(In reply to Justin Lebar [:jlebar] from comment #1)
> Okay, so this regression is due to a change in how we handle shared
> libraries.
> 
> Glandium, I see a bunch of entries in smaps like
> 
>   72000000-72074000 r-xs 0 00:04 982065     /dev/ashmem/libxul.so (deleted)
> 
> Do you know why it says "(deleted)"?  Smaps claims the mapping is indeed
> taking up resident space...

It does this because the file actually doesn't exist on the file system. It actually never existed, so it was never deleted per se, but the kernel code that displays "(deleted)" makes assumptions.

This is how ashmem works, unfortunately. Note this didn't really change. What changed is that before you could either have this behaviour (decompress in memory and link the memory region) or another (uncompressing the files and dlopen() them), now you can only have this one.
(Reporter)

Comment 3

7 years ago
I see; cool.  I'll fix up the smaps parsing code so it understands this.  Thanks!
(Reporter)

Comment 4

7 years ago
Created attachment 565931 [details] [diff] [review]
Patch v1
Attachment #565931 - Flags: review?(khuey)
Comment on attachment 565931 [details] [diff] [review]
Patch v1

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

::: xpcom/base/MapsMemoryReporter.cpp
@@ +99,5 @@
>  }
>  
>  void GetBasename(const nsCString &aPath, nsACString &aOut)
>  {
> +  nsCString out;

Why do you need the temporary?  Does nsACString not have StripChars or RFind or something?
Attachment #565931 - Flags: review?(khuey) → review+
(Reporter)

Comment 6

7 years ago
> Why do you need the temporary?  Does nsACString not have StripChars or RFind or something?

Yes, bizarrely.
(In reply to Justin Lebar [:jlebar] from comment #6)
> > Why do you need the temporary?  Does nsACString not have StripChars or RFind or something?
> 
> Yes, bizarrely.

I'm not surprised.  Our string api is insane.
https://hg.mozilla.org/mozilla-central/rev/8e85383d821e
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
(Reporter)

Updated

7 years ago
Depends on: 693976
You need to log in before you can comment on or make changes to this bug.