http://codereview.chromium.org/2310003/diff/1/4 File src/isolate.h (right):
http://codereview.chromium.org/2310003/diff/1/4#newcode40 src/isolate.h:40: static Isolate* current(); On 2010/05/28 14:59:55, Vitaly wrote:
I think we should only allow lazy initialization in the API layer.
Internally we
should be able to assume the current isolate already exists. One of
the reasons
is that getting the current isolate is a performance critical thing.
I don't think the isolate should ever be lazily initialized.
Comment on naming: Isolate::current() makes the reader think it's so
fast that
it doesn't really make sense to cache it in a local variable. While
we'd like
this to be the case, it still will be slower than that. So
Isolate::GetCurrent()
is a better name.
What Vitaly is referring to is http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Function_Names#Function_Names Accessing the current isolate is not a trivial accessor, but I would suggest that you call the method Current() to match the style guide. The "Get" part is unnecessary. http://codereview.chromium.org/2310003/diff/1/5 File src/v8.cc (right): http://codereview.chromium.org/2310003/diff/1/5#newcode56 src/v8.cc:56: return Isolate::current()->InitializeFromInternalV8Temp(des); On 2010/05/28 14:59:55, Vitaly wrote:
What about initialization that shouldn't be done for each isolate?
E.g.
OS::Setup(), CPU features probing?
We should split initialization into global and per-isolate functions.
I would suggest to add a static Isolate* Init(Deserializer* des) to the Isolate class. This would solve the lazy initialization issue that was raised in Isolate::Current() and gets rid of the InitializeFromInternalV8Temp() call. In addition the "friend class V8" in isolate.h is obsolete as well. As far as once-per-process initialization goes I suggest that the different sub-systems add a static InitOnce in these classes and call them from a central place. For example I expect that you will need an Isolate::InitOnce() method down the road. http://codereview.chromium.org/2310003/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
