ClosestStorm / v8cgi

Automatically exported from code.google.com/p/v8cgi
BSD 3-Clause "New" or "Revised" License
0 stars 1 forks source link

slow GC for Buffer instances (was: crash in ('binary').Buffer... or somewhere near) #88

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
When running attached test and v8/v8cgi are both compiled in debug mode, v8cgi 
will crash on assert inside garbage collection.

=====

$ v8cgi --gc-interval=0 -- crash.js

#
# Fatal error in src/global-handles.cc, line 186
# CHECK(state_ != NEAR_DEATH) failed
#

(... lots of stack/heap information ...)

Aborted (core dumped)
=====

I am not familiar with V8 embedding, most of my work is on spidermonkey. It 
looks a bit similar to situation when I forget to root variable in SM, but it 
could be bug in V8 itself, too - I am reporting here first, because I couldn't 
reproduce it without involving binary.Buffer in some way. I have tried 
commenting out various parts of the ClientResponse function and replacing them 
with constants, but can't make anything usable out of it. Some combinations 
work, some do not - it's not any single function that fails.

When I run this in release builds, or when I remove the assert from V8, the 
simplified script runs to the end. But
a) usually, asserts are in code for good reason
b) in the full version of the script, the garbage collection quickly starts to 
take progressively more time - from 1-2 minutes after couple thousands http 
requests, up to more than 30 minutes of eating 100% processor without any 
progress, when it gets to the 60K+ numbers (I didn't have enough patience after 
that and canceled it). Memory usage slowly grows too, even when I have all the 
data already in memory from the start, and delete them after they are no longer 
needed. Those issues should be related to the ignored assert, since I started 
from there and worked towards the minimal example.

Original issue reported on code.google.com by hua...@gmail.com on 14 Feb 2011 at 8:31

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks a lot for submitting, I will look into it. Have you tried with latest 
svn revision of v8cgi?

Original comment by ondrej.zara on 14 Feb 2011 at 9:00

GoogleCodeExporter commented 9 years ago
> Have you tried with latest svn revision of v8cgi?

Yes, most of the testing was done on current HEAD of both v8cgi and v8 (r906 
and r6757). Right now I did quick check with v8cgi-0.9.0-src.tar.gz just to be 
sure, and the behaviour is the same.

Original comment by hua...@gmail.com on 14 Feb 2011 at 9:33

GoogleCodeExporter commented 9 years ago
In the meantime, I was able to reproduce the crash. Debugging it might be 
harder, though :)

Original comment by ondrej.zara on 14 Feb 2011 at 10:31

GoogleCodeExporter commented 9 years ago
Reduced testcase.

Original comment by ondrej.zara on 14 Feb 2011 at 11:11

Attachments:

GoogleCodeExporter commented 9 years ago
The issue is certainly related to the way v8cgi handles disposal of weak 
persistent handles. More specifically, the crash occurs when the temporary 
buffer (created using ".range()") gets garbage collected.

Original comment by ondrej.zara on 14 Feb 2011 at 11:13

GoogleCodeExporter commented 9 years ago
Please try updating to revision 907, recompiling and testing again. I believe I 
fixed the bug.

Original comment by ondrej.zara on 14 Feb 2011 at 1:07

GoogleCodeExporter commented 9 years ago
Thanks for the quick response and for the time you spent on this. The reduced 
cases and crash appear to be fixed, but the original issue is not, looks like 
they were not as related as I had hoped.

I did some more measurement, and after some large number of http queries, the 
garbage collector still goes insane and for extended periods of time it sits in 
a tight loop, eating 100% of processor, without any progress with the script. 
The number probably depends on the amount of data exchanged. In the attached 
case it gets to 13000, next stop (after few minutes) is at 25k and 40k - I 
didn't have patience to let it work past that. With real data, after 14k 
queries the script is as good as dead.

I think this issue can be closed, unless you want to look further into it. This 
may even be the intended performance characteristics of the GC, and in any 
case, doing so many separate queries is wrong approach to my problem, I was 
hoping to only use it only as quick and dirty solution and I'd have to optimize 
it later anyway. If nothing else, it would be dying on system limit for number 
of TIME_WAIT sockets, the GC trouble just delays the execution long enough for 
them to die.

Original comment by hua...@gmail.com on 16 Feb 2011 at 8:15

Attachments:

GoogleCodeExporter commented 9 years ago
I will look into this more thoroughly: statistically speaking, the issue is 
*almost certainly* caused by my bad api/c++ usage and not by V8 itself. I will 
just rename the problem a bit, as the crash was successfully remedied.

Thanks for the torture script; are there any recommended command-line switches 
(such as the gc-interval) for running it?

Original comment by ondrej.zara on 16 Feb 2011 at 8:44

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
I just noticed that binary doesn't always throw errors when it receives bad 
input.
Here's a test + patch for it.

Original comment by nik...@gmail.com on 5 May 2011 at 10:40

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks a lot, commited in r912.

Original comment by ondrej.zara on 6 May 2011 at 10:48

GoogleCodeExporter commented 9 years ago
Run this .js file in the background and watch it slowly use up lots of memory.
Use the patch and run it again, the garbage collector will do it's work and it 
won't run out of memory.

You may want to use AdjustAmountOfExternalAllocatedMemory  where ever else you 
need to allocate memory.

Original comment by nik...@gmail.com on 10 May 2011 at 1:09

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks a lot for this patch! It indeed solves the issue; torture2.js no longer 
hangs up. I assume this issue can be considered finally fixed.

Commited in r914.

Original comment by ondrej.zara on 13 May 2011 at 5:40