On 2013/12/03 17:07:16, titzer wrote:
On 2013/12/03 15:47:02, titzer wrote:
> On 2013/12/03 15:27:32, Michael Starzinger wrote:
> > Not LGTM, adding Ben as a reviewer.
> >
> > Ben spent several hours making sure that OSR code is _not_ installed
in
> > JSFunction objects. This change completely defeats and breaks that
invariant,
> by
> > bypassing the choking point that was introduced. I agree that we might
want
to
> > cache OSR code, but we don't want to install it in JSFunction objects.
> >
> >
>
https://codereview.chromium.org/100613004/diff/20001/src/code-stubs-hydrogen.cc
> > File src/code-stubs-hydrogen.cc (right):
> >
> >
>
https://codereview.chromium.org/100613004/diff/20001/src/code-stubs-hydrogen.cc#newcode1195
> > src/code-stubs-hydrogen.cc:1195: // case, it's probably a good idea to
just
> > reuse that OSR code right now.
> > I strongly disagree that this is a good idea! Once Ben's work is
done, it
will
> > be incorrect to do that. This will just take whatever code is in the
> > optimized_map and smash it into the JSFunction. OSR code should not be
> installed
> > into JSFunctions at all.
>
> By the way, I just remembered that we have hydrogenized code that
checks the
> optimized code map something like FastNewClosureStub; that's bad since
even
if
> you fix it here, that stub might find OSR code.
>
> I'm kind of thinking you might want a separate cache of OSR-compiled
code,
which
> we sort of already have hanging around in the compiler itself in the
queues
> between the compiler thread and the main thread. I suggest repurposing
that
to
> cache the OSR code for a limited time.
>
> IIRC the optimized code map is cleared by the GC at various times, so
the
> lifetime of that cache should be similar.
As Michael pointed out, I am retarded. This comment is _in_ the callee of
this
function. :)
Uploaded a new patch set to address the issues.
I'm still using the optimized code map. The rationale is that the OSR
buffer in
the optimizing compiler thread does not map shared function info and native
context to code, but function objects to compile jobs. Compile jobs are not
yet
generated code and contain keeps a lot of memory alive.
I added a 4th slot to the optimized code map for the osr ast id. I also
added
code to the hydrogen stub to ignore those that have an osr ast id set. On
Runtime_CompileForOnStackReplacement I check for cached code before I do
everything. Since I need the osr ast id at that point already, I just
removed
the whole thing of using pc_offset as key in RecompileJob.
https://codereview.chromium.org/100613004/
--
--
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.
