Closed Bug 693101 Opened 8 years ago Closed 8 years ago

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

Categories

(Toolkit :: about:memory, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla10

People

(Reporter: justin.lebar+bug, Unassigned)

References

Details

(Whiteboard: [inbound])

Attachments

(1 file)

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.
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.
I see; cool.  I'll fix up the smaps parsing code so it understands this.  Thanks!
Attached patch Patch v1Splinter Review
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+
> 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
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
Depends on: 693976
You need to log in before you can comment on or make changes to this bug.