plack / Plack

PSGI toolkit and server adapters
http://plackperl.org/
Other
486 stars 214 forks source link

"plackup shotgun" consume 1.2 GB of RAM #400

Closed DNS closed 11 years ago

DNS commented 11 years ago

after running about 4-6 hours, "Plackup -L Shotgun" consume about 1.2GB of memory

step to reproduce:

 plackup -L Shotgun --host localhost --port 80 test.pl
miyagawa commented 11 years ago

Can't reproduce.

plackup -L Shogun ~/dev/Plack/eg/dot-psgi/Hello.psgi

with sending 10K requests to localhost:5000 and the memory usage is steady.

Does your test application fork? Can you reproduce with a simple Hello World app?

DNS commented 11 years ago

seems someone else also encountered this problem (with Shotgun): http://www.perlmonks.org/index.pl?node_id=1002861

yes, I use simple hello world app on windows (both simple Mojo & Dancer app) always can reproduce this bug. Each request always increase plackup memory usage (mojo +8MB, dancer +4.8MB for each http request). and can quickly consume more than 600MB

However, I tested on Fedora, but I can't reproduce this kind of bug. Memory consumption always steady.

probably OS-related bug (I'm running on win7-64)

P.S: maybe related to this: https://github.com/plack/Plack/issues/320 ?

miyagawa commented 11 years ago

Yeah, sounds like the issue is limited to Win32.

miyagawa commented 11 years ago

because of the way Shotgun forks off a subprocess to let it handle the request and then quit, it's not hard to imagine that it doesn't work cleanly with Win32.

Unless someone can point out the way to fix it to work on Win32 without a memory leak, I would patch Shotgun to die on Win32 saying it's not supported. Maybe one can write a different loader using CreateProcess() system call for win32 to get equivalent.

frioux commented 11 years ago

I've done a few hours of research for perl serving on win32 and I've come to the conclusion that the only good solution is to just start completely separate processes and use something like apache to load balance them. If you were to use threads instead of forks it would buy the user nothing really (no CoW for RAM savings), and they'd still have a SPOF if the process were to get corrupted somehow. I vote die.

miyagawa commented 11 years ago

@frioux Hm, not sure if I understand correctly - Shotgun isn't about load balancing them like Starman/Starlet, it is to fork off a separate process for each request (in-process, much like CGI) and shuts it down.

"no CoW for RAM savings" is exactly what you need here, since the goal is to not share the loaded module-space to make a fresh perl process. It's not about the performance it's about the refreshing/restarting during the development. It probably eliminates the benefit of using -M to preload rarely modified modules in the parent process for CoW, but that's acceptable.

frioux commented 11 years ago

Oh sorry, I didn't understand, clearly.

jandubois commented 11 years ago

The fact that the fork() emulation on Windows leaks memory is probably a bug. But in my experience it is never a good idea to use fork() emulation anyways, so I'm not really motivated to look for the cause. :)

You could use my $pid = system(1, @args); on Windows to spawn another process asynchronously. This is somewhat underdocumented, but it is mentioned in perlport.pod. You'll still have to call waitpid() on it eventually; there is a limit of around 60 processes you can spawn like this simultaneously. The same limit applies to fork() emulation as well (and they use the same array to keep the child information, so if you have 30 forked children, then you can only spawn another 30 processes with system(1,...) until you have to start reaping them).

miyagawa commented 11 years ago

OK, until someone volunteers to provide alternative implementation of Loader that works safe on win32, this is closed with the dying message.

DNS commented 11 years ago

actually this is not memory leak. The windows kernel can claim the memory if the perl plackup -L Shotgun process killed.

I know how to fix this, there are several alternatives:

a) work closely with perl5-porters: reimplement fork() on windows by using JMP & other neat instructions by disassembling a running forking program on linux box (need low level kernel debugger on linux box, but I don't know there's low level debugger on linux exist yet since they already have the kernel source)

b) suggest cygwin fork() as an alternative to perl5-porters (this is so much easier than (A), but I doubt they have/use windows box)

c) rather than using perl fork, plack can use threads and works nicely with all platforms. But may need major-medium changes to current Plack source code structure.

d) implement Shotgun with native windows threads, maybe CreateThread() or using existing perl win32 modules from cpan ?

I think (D) is much more easier. I'll try to send patch this weeks.

miyagawa commented 11 years ago

actually this is not memory leak. The windows kernel can claim the memory if the perl plackup -L Shotgun process killed.

If fork() and then shutting down the forked children can't reclaim memory inside perl, i would call it a memory leak.

I think (D) is much more easier. I'll try to send patch this weeks.

given the size of the Shotgun code, i would suggest you to create a new implementation, such as Plack::Loader::ShotgunWin32 (or whatever creative name you can come up with), and upload to CPAN as a separate distribution, rather than a patch.

DNS commented 11 years ago

https://github.com/dns/Plack/blob/master/lib/Plack/Loader/Shotgun.pm

I've fixed this, works well on windows using perl threads, not limited to windows but I think also cross platfrom.

miyagawa commented 11 years ago

You can release it as a separate CPAN distribution using a different name (and modified POD to reflect that it uses thread).

I don't trust perl thread on UNIX systems, at least not as fork(2), so i'm not gonna merge that, but feel free to fork your own :)