Reviewers: Søren Gjesse,
Message:
Made some big changes in d8:
- ported the --isolate option from the sample shell. There are some
differences
because the sample shell does not use lockers.
- moved the entire parsing of command line options to a stand-alone method
(SetOptions), keeping the parsed result in a passive static object
(Shell::options)
http://codereview.chromium.org/7318002/diff/1/src/d8.h
File src/d8.h (right):
http://codereview.chromium.org/7318002/diff/1/src/d8.h#newcode115
src/d8.h:115: class SourceGroup {
class necessary for the --isolate option
http://codereview.chromium.org/7318002/diff/1/src/d8.h#newcode173
src/d8.h:173: class ShellOptions {
grouped all the options for the shell in here.
http://codereview.chromium.org/7318002/diff/1001/src/d8.cc
File src/d8.cc (right):
http://codereview.chromium.org/7318002/diff/1001/src/d8.cc#newcode45
src/d8.cc:45: #include <unistd.h> // NOLINT
necessary for _exit()
http://codereview.chromium.org/7318002/diff/1001/src/d8.cc#newcode633
src/d8.cc:633: void Shell::Initialize() {
test_shell is now a part of Shell::options and therefore static
http://codereview.chromium.org/7318002/diff/1001/src/d8.cc#newcode676
src/d8.cc:676: context_mutex_->Lock();
mark critical section because creating a context is not thread-safe
http://codereview.chromium.org/7318002/diff/1001/src/d8.cc#newcode724
src/d8.cc:724: v8::Unlocker unlocker(Isolate::GetCurrent());
the default constructor takes the default isolate, which is *not* the
same as the current isolate.
http://codereview.chromium.org/7318002/diff/1001/src/d8.cc#newcode824
src/d8.cc:824: // Prepare the context for this thread.
no point in locking earlier than necessary
http://codereview.chromium.org/7318002/diff/1001/src/d8.cc#newcode853
src/d8.cc:853:
reused code from sample/shell.cc
http://codereview.chromium.org/7318002/diff/1001/src/d8.cc#newcode936
src/d8.cc:936: Locker lock(isolate);
the default constructor takes the default isolate, which is *not* the
same as the current isolate. therefore we explicitly use the new
isolate. otherwise the locker will actually enter the default isolate,
making this whole construct pointless.
http://codereview.chromium.org/7318002/diff/1001/src/d8.cc#newcode971
src/d8.cc:971: bool Shell::SetOptions(int argc, char* argv[]) {
moved all the options parsing into this. the parsed options are then
stored in Shell::options
http://codereview.chromium.org/7318002/diff/1001/src/d8.cc#newcode1033
src/d8.cc:1033: printf("-p is not compatible with --isolate\n");
either we have parallelism with -p or with --isolate, but not both! the
difference is that -p works on the same isolate.
http://codereview.chromium.org/7318002/diff/1001/src/d8.cc#newcode1077
src/d8.cc:1077: if (options.parallel_files != NULL)
if parallel_files != NULL then there obviously are no --isolate options
http://codereview.chromium.org/7318002/diff/1001/src/d8.cc#newcode1096
src/d8.cc:1096: options.isolate_sources[0].Execute();
this part executes even if single-threaded.
http://codereview.chromium.org/7318002/diff/1001/src/d8.h
File src/d8.h (right):
http://codereview.chromium.org/7318002/diff/1001/src/d8.h#newcode220
src/d8.h:220: static Persistent<Context> CreateEvaluationContext();
renamed this to reflect that it's used in several occasions
Description:
ported --isolate option to d8 and refactored to group together option
parsing
TEST=tools/test.py -j15 --shell d8 --isolates
Please review this at http://codereview.chromium.org/7318002/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/d8.h
M src/d8.cc
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
