Closed Bug 1527597 Opened 1 year ago Closed 4 months ago

Firefox on Solaris requires too much space on swap for reservations

Categories

(Core :: JavaScript Engine, defect, P5)

60 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox71 --- fixed

People

(Reporter: petr.sumbera, Assigned: petr.sumbera)

References

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:65.0) Gecko/20100101 Firefox/65.0

Steps to reproduce:

Linux unlike Solaris lets you over-commit swap reservations, and doesn't
actually allocate it until it's usedt.

On Solaris Firefox by default starts with 3 processes and each one eats almost 1.5 gigabytes of memory on swap. Where 1 gigabyte per process are only swap reservations.

Before start:

$ swap -sh
total: 1.5G allocated + 724M reserved = 2.2G used, 6.7G available

After start:

$ swap -sh
total: 1.9G allocated + 3.7G reserved = 5.6G used, 3.3G available

top command shows:

PID USERNAME NLWP PRI NICE SIZE RES STATE TIME CPU COMMAND
2770 root 17 49 0 1475M 261M run 0:11 7.77% firefox
2768 root 52 59 0 1457M 239M sleep 0:15 1.50% firefox
2771 root 17 59 0 1296M 86M sleep 0:00 0.00% firefox

Therefore we need to have big enough swap (at least 8 gigabytes).

Or you we can limit Firefox to just one process by setting about:config option browser.tabs.remote.autostart to false.

Is there some other way to limit swap reservations?

I believe the problem was already discussed here:
https://bugzilla.mozilla.org/show_bug.cgi?id=1496713#c17

Component: Untriaged → JavaScript Engine
Product: Firefox → Core
Priority: -- → P5

It seems that with Firefox 68 the situation got much worser now. I mean each tab has its own process.

Luke, can you please eventually suggest where to use MAP_NORESERVE and what else should I try to change to resolve this?

Flags: needinfo?(luke)

I don't know; perhaps Eric can find someone who does?

Flags: needinfo?(luke) → needinfo?(erahm)

Hi Petr, you can use searchfox to find where we use PROT_NONE. If I understand correctly you probably care about MapBufferMemory. I hope that helps, it'll be interesting to know if this works for you!

Flags: needinfo?(erahm)

Yes, I'd try what erahm is suggesting.

Are there ways to throw away physical pages and the swap reservation on Solaris? On Linux (not that it reserves swap) you can do mprotect(PROT_NONE) then mprotect(PROT_READ | PROT_WRITE) and it'll throw away the physical pages. On windows you can VirtualAlloc the memory without the MEM_COMMIT flag to reserve the virtual memory address ranges without allocating swap or physical pages.

Plus I'm working on Bug 1564674 which will avoid reserving swap space for some areas on Windows. After that lands if you want to add special #ifdef'd paths for Solaris that'd be great.

Thanks.

you can do mprotect(PROT_NONE) then mprotect(PROT_READ | PROT_WRITE) and it'll throw away the physical pages.

I doubt it does. madvise(MADV_DONTNEED) does.

Thank you all for reply! I think I found working fix:

--- firefox-68.0/js/src/jit/ProcessExecutableMemory.cpp
+++ firefox-68.0/js/src/jit/ProcessExecutableMemory.cpp
@@ -344,7 +344,7 @@
   // mmap will pick a different address.
   void* randomAddr = ComputeRandomAllocationAddress();
   void* p = MozTaggedAnonymousMmap(randomAddr, bytes, PROT_NONE,
-                                   MAP_PRIVATE | MAP_ANON, -1, 0,
+                                   MAP_NORESERVE | MAP_PRIVATE | MAP_ANON, -1, 0,
                                    "js-executable-memory");
   if (p == MAP_FAILED) {
     return nullptr;

Note that with every new process ReserveProcessExecutableMemory reserved 0x7FC00000 on swap. I have now no problem with small swap and Firefox seem to run well.

Any comment to this? Can we add this to Firefox also for other systems? Or just for Solaris? Thank you!

Probably just for Solaris, I'd like to know how the others behave before adding extra parameters.

I'm concerned that if this array is later written in, and page faults cause pages to be allocated, is the swap reserved at that point or is it never reserved until its needed. The manpage doesn't say. Then we might need a way to tell the OS that now would be a good time to reserve some swap space for the section of this mapping that we're touching/about to touch. If you happen to know then I'd be very interested.

I'm probably overthinking it. If this one line change makes things better on Solaris then I'm happy with it in some XP_SOLARIS #ifdefs. Petr do you want to write the patch?

(In reply to Paul Bone [:pbone] from comment #9)

I'm concerned that if this array is later written in, and page faults cause pages to be allocated, is the swap reserved at that point or is it never reserved until its needed. The manpage doesn't say. Then we might need a way to tell the OS that now would be a good time to reserve some swap space for the section of this mapping that we're touching/about to touch. If you happen to know then I'd be very interested.

I'm told that it'll make the reservation when the page is committed (when it is touched) so this isn't a problem and I was indeed overthinking.

FWIW, this is the documentation for MAP_NORESERVE on Linux:

   MAP_NORESERVE
         Do  not  reserve swap space for this mapping.  When swap space is reserved, one has
         the guarantee that it is possible to modify the mapping.  When swap  space  is  not
         reserved  one  might  get  SIGSEGV upon a write if no physical memory is available.
         See also the discussion of the file /proc/sys/vm/overcommit_memory in proc(5).   In
         kernels before 2.6, this flag had effect only for private writable mappings.

(In reply to Paul Bone [:pbone] from comment #10)

(In reply to Paul Bone [:pbone] from comment #9)
I'm told that it'll make the reservation when the page is committed (when it is touched) so this isn't a problem and I was indeed overthinking.

So can we make this default?

Keywords: checkin-needed

Pushed by rmaries@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8cc5f974bbd0
ReserveProcessExecutableMemory should use MAP_NORESERVE r=jandem

Keywords: checkin-needed
Status: UNCONFIRMED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
Assignee: nobody → petr.sumbera
Regressions: 1589975
You need to log in before you can comment on or make changes to this bug.