LGTM Regarding the lazy compilation the following code should ensure that a function compiled through th API is actually compiled.
v8::Handle<v8::Function> fun ... Handle<v8::internal::JSFunction> f = v8::Utils::OpenHandle(*fun); Handle<v8::internal::SharedFunctionInfo> shared(f->shared()); if (!shared->is_compiled()) ok = CompileLazyShared(shared, CLEAR_EXCEPTION, 0); Regards, Søren On Wed, Feb 25, 2009 at 4:40 PM, <[email protected]> wrote: > > http://codereview.chromium.org/28112/diff/1/3 > File src/log.h (right): > > http://codereview.chromium.org/28112/diff/1/3#newcode276 > Line 276: class StackTracer { > On 2009/02/25 12:36:13, Søren Gjesse wrote: > >> Please add BASE_EMBEDDED. >> > > Done. > > http://codereview.chromium.org/28112/diff/1/5 > File test/cctest/test-log-ia32.cc (right): > > http://codereview.chromium.org/28112/diff/1/5#newcode141 > Line 141: static Handle<JSFunction> Compile(const char* source) { > On 2009/02/25 12:36:13, Søren Gjesse wrote: > >> For compiling JavaScript I think you should stick to using the public >> > API when > >> possible instead of using internal functions, e.g. CompileFunction in >> test-debug.cc. You can always later convert API objects to their >> > internal > >> counterparts when required. >> > > Thanks to your suggestion I removed dozen lines of code. I simply didn't > knew about OpenHandle / ToApi utilities before. > > http://codereview.chromium.org/28112/diff/1/5#newcode150 > Line 150: static void CompileRun(const char* source) { > On 2009/02/25 12:36:13, Søren Gjesse wrote: > >> Again use the public API for this. >> > > Done. > > http://codereview.chromium.org/28112/diff/1/5#newcode208 > Line 208: original, patch, sizeof(patch))); > On 2009/02/25 12:36:13, Søren Gjesse wrote: > >> This code patching seems risky and changes to the compiler might break >> > it. How > >> about adding a runtime function to return the FP of the caller? >> > > OK, maybe we will get rid of this in the next iteration. E.g. I didn't > know about inline runtime functions. > > http://codereview.chromium.org/28112/diff/1/5#newcode210 > Line 210: SetGlobalProperty("JSFuncDoTrace", *call_trace); > On 2009/02/25 12:36:13, Søren Gjesse wrote: > >> Look at CompileFunction in test-debug.cc to compile 'function >> > JSFuncDOTrace() { > >> ... }'. >> > > > I tried this already, but because of lazy compilation, I haven't got a > real compiled code but instead a stub that calls LazyCompile. And > running the function prior to patching is not an option here. > > http://codereview.chromium.org/28112/diff/1/5#newcode212 > Line 212: SetGlobalProperty("sample", Smi::FromInt(PtrToSmi(&sample))); > On 2009/02/25 12:36:13, Søren Gjesse wrote: > >> I don't see the need to store these values in the JS global object. >> > Can't they > >> just be global variables in C++? They are only used from C++ code in >> > this file. > > Good idea! Done. > > > http://codereview.chromium.org/28112 > --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
