All comments addressed.

http://codereview.chromium.org/2632003/diff/9001/10001
File src/compilation-cache.cc (right):

http://codereview.chromium.org/2632003/diff/9001/10001#newcode219
src/compilation-cache.cc:219:
CompilationCacheTable::cast(tables_[generation]);
On 2010/06/07 08:31:11, Mads Ager wrote:
Four-space indent.

Done.

http://codereview.chromium.org/2632003/diff/9001/10001#newcode221
src/compilation-cache.cc:221: Object* object =
table->Lookup(String::cast(script->source()));
On 2010/06/07 08:31:11, Mads Ager wrote:
Extract the source before the loop?

Done.

http://codereview.chromium.org/2632003/diff/9001/10002
File src/compilation-cache.h (right):

http://codereview.chromium.org/2632003/diff/9001/10002#newcode83
src/compilation-cache.h:83: static bool HasFunction(SharedFunctionInfo*
sfi);
On 2010/06/07 08:31:11, Mads Ager wrote:
I like function_info better than sfi.
Changed everywhere

http://codereview.chromium.org/2632003/diff/9001/10004
File src/heap.cc (right):

http://codereview.chromium.org/2632003/diff/9001/10004#newcode2190
src/heap.cc:2190: // the code contains the stack pointer from any of the
stacks.
On 2010/06/07 08:31:11, Mads Ager wrote:
It is the other way around:

to see if there are activations on any of the stack corresponding to
the code.

Done.

http://codereview.chromium.org/2632003/diff/9001/10004#newcode2211
src/heap.cc:2211: void FlushFunction(SharedFunctionInfo* sfi) {
On 2010/06/07 08:31:11, Mads Ager wrote:
FlushFunction -> FlushCodeForFunction?

Done.

http://codereview.chromium.org/2632003/diff/9001/10004#newcode2216
src/heap.cc:2216: // We never delete Api functions.
On 2010/06/07 08:31:11, Mads Ager wrote:
delete -> flush code for

Done.

http://codereview.chromium.org/2632003/diff/9001/10004#newcode2219
src/heap.cc:2219: // This must be a function type.
On 2010/06/07 08:31:11, Mads Ager wrote:
Only flush code for functions.

Done.

http://codereview.chromium.org/2632003/diff/9001/10004#newcode2225
src/heap.cc:2225: // If this function is in the compilation cache we
don't remove it.
On 2010/06/07 08:31:11, Mads Ager wrote:
don't remove it -> do not flush the code.

Done.

http://codereview.chromium.org/2632003/diff/9001/10004#newcode2245
src/heap.cc:2245: // Do not flush code if debug is loaded.
On 2010/06/07 08:31:11, Mads Ager wrote:
debug is loaded -> the debugger is loaded?

Done.

http://codereview.chromium.org/2632003/diff/9001/10004#newcode2251
src/heap.cc:2251: // Don't remove unnamed functions.
On 2010/06/07 08:31:11, Mads Ager wrote:
I don't think I understand why we cannot flush the code for anonymous
functions?
Could you explain why or allow it? :)
As discussed offline this caught the cases where a complete script is
encapsulated in a function if compiled using Script::Compile. This has
been removed and a check for is_toplevel() is included in the
FlushFunction method. This allows us to also flush code for anonymous
functions.

http://codereview.chromium.org/2632003/diff/9001/10005
File src/heap.h (right):

http://codereview.chromium.org/2632003/diff/9001/10005#newcode1269
src/heap.h:1269: static void FlushCode();
On 2010/06/07 08:31:11, Mads Ager wrote:
Please add a comment.

Done.

http://codereview.chromium.org/2632003/show

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to