On 2013/09/24 10:07:11, Yang wrote:

https://codereview.chromium.org/24237009/diff/3001/src/optimizing-compiler-thread.cc
File src/optimizing-compiler-thread.cc (right):


https://codereview.chromium.org/24237009/diff/3001/src/optimizing-compiler-thread.cc#newcode186
src/optimizing-compiler-thread.cc:186: FlushOutputQueue(false);
On 2013/09/24 09:42:01, titzer wrote:
> Shouldn't you also empty the osr buffer here too?

This is already done in FlushOutputQueue.


https://codereview.chromium.org/24237009/diff/3001/src/optimizing-compiler-thread.cc#newcode304
src/optimizing-compiler-thread.cc:304: delete info;
On 2013/09/24 09:42:01, titzer wrote:
> Maybe you want DisposeOptimizingCompiler(stale, false)? That way, all the
> deletes go through the same code path.

Done.


https://codereview.chromium.org/24237009/diff/3001/src/optimizing-compiler-thread.h
File src/optimizing-compiler-thread.h (right):


https://codereview.chromium.org/24237009/diff/3001/src/optimizing-compiler-thread.h#newcode122
src/optimizing-compiler-thread.h:122: List<OptimizingCompiler*>
osr_candidates_;
On 2013/09/24 09:42:01, titzer wrote:
> First, I am not even sure why you need two buffers here.
>
> Second, It looks like this two buffer scheme can leak OptimizingCompilers.
You
> are using the osr_candidates_ as a list of the OSR compilation requests.
When
> the compilation request finishes, it gets moved from the osr_candidates_ to
the
> osr_buffer_, the latter of which is a fixed-size buffer. But that buffer
holds
> onto those OptimizingCompilers until they are either removed by
> FindReadyOsrCandidate or until they are overwritten by the next
AddToOsrBuffer.
> That's a problem, since an Optimizing compiler holds onto potentially a lot
of
> memory--the entire hydrogen and lithium graphs, I think.

osr_candidates_ simply mirrors OSR jobs that are either still in the
input_queue_, being processed by CompileNext(), or waiting in the
output_queue_.
Once a job ends up in the osr_buffer_, it's removed from osr_candidates_.
Assuming that the recompile loop doesn't block somewhere, it always gets
removed
from osr_candidates_ eventually. It's used to check whether we already have a
certain OSR job (function x ast id), since the back edge state is not 100%
reliable for this purpose.

I think you can do this with just one list of the OSR-related compilation
requests. They just stay in the list until they are pulled out by a successful
FindReadyOsrCandidate.


The problem of osr_buffer_ holding onto OptimizingCompiler objects is
unrelated
to this. It seems unavoidable since we don't know when or whether a completed
OSR job actually gets used for OSR entry, so we have to have a number of
recent
ones waiting. The best solution I could think of for now is to somehow install hooks at the point when we enter optimized code for a function to remove all
other waiting OSR jobs for that function. For now all we do is flushing
osr_buffer_ at context (V8::Context) change.

Yes, it's a separate problem that is independent of whether there are two
buffers. There either needs to be some kind of aging mechanism or timeout or
something. You suggested using the low memory notification; that only works if the system is running out of memory. If it isn't, then these other OSR compiles
will just lay around forever taking up memory. We need to come up with a
solution for that...

https://codereview.chromium.org/24237009/

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to