Reviewers: danno, Benedikt Meurer,
https://codereview.chromium.org/75283002/diff/1/build/features.gypi
File build/features.gypi (right):
https://codereview.chromium.org/75283002/diff/1/build/features.gypi#newcode63
build/features.gypi:63: 'v8_use_default_platform%': 1,
On 2013/11/18 15:34:44, Benedikt Meurer wrote:
Do we really need this option here? Couldn't we just always include
our default
implementation, and in V8::Initialize just check whether g_platform is
NULL, and
if so, automagically use the default implementation?
The default implementation will have to provide e.g. a thread pool, so
it's a non-trivial amount of code which I'd like to avoid including if
possible.
Also, V8::Initialize just picking a default implementation is too much
magic IMO. It would lead to unexpected results if we e.g. add a new
process in chrome and forget to set the platform there.
https://codereview.chromium.org/75283002/diff/1/include/v8-platform.h
File include/v8-platform.h (right):
https://codereview.chromium.org/75283002/diff/1/include/v8-platform.h#newcode51
include/v8-platform.h:51: class Platform {
On 2013/11/18 15:34:44, Benedikt Meurer wrote:
Add private copy constructor and assign operator, using V8_DELETE.
It's a pure virtual interface, there's no point in doing so?
https://codereview.chromium.org/75283002/diff/1/include/v8-platform.h#newcode64
include/v8-platform.h:64: virtual void callOnBackgroundThread(Task*
task, bool task_is_slow) = 0;
On 2013/11/18 15:34:44, Benedikt Meurer wrote:
Nit: First letter should be upper-case.
Done.
https://codereview.chromium.org/75283002/diff/1/include/v8-platform.h#newcode71
include/v8-platform.h:71: virtual void callOnForegroundThread(Isolate*
isolate, Task* task) = 0;
On 2013/11/18 15:34:44, Benedikt Meurer wrote:
Nit: First letter should be upper-case.
Done.
https://codereview.chromium.org/75283002/diff/1/src/api.cc
File src/api.cc (right):
https://codereview.chromium.org/75283002/diff/1/src/api.cc#newcode5087
src/api.cc:5087: g_platform = new i::DefaultPlatform;
On 2013/11/18 15:34:44, Benedikt Meurer wrote:
I think platform initialization should be first in V8::Initialize().
If we
really want to be able to move all the platform stuff to the Platform
class at
some point, then we will need an initialized Platform to use even
basic stuff
such as TLS. So, please do:
if (!g_platform) g_platform = new i::DefaultPlatform;
at the beginning.
Done (except for the default magic)
Description:
Introduce a v8::Platform class that bundles embedder callbacks
Also provide a default implementation to use in cctests.
For now, there are just two thread releated callbacks. In future CLs, I will
move callbacks registered e.g. via V8::SetFooCallback over.
BUG=v8:3015
[email protected]
Please review this at https://codereview.chromium.org/75283002/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+131, -33 lines):
M build/features.gypi
A + include/v8-platform.h
M src/api.cc
A + src/default-platform.h
A + src/default-platform.cc
M src/v8.h
M tools/gyp/v8.gyp
--
--
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.
