Introduces a light version of D8 that links against shared library.

Introduces a light version of D8 that links against shared library.

Committed: http://code.google.com/p/v8/source/detail?r=8657

[Yang, 2011-07-13 13:26:10]

The idea is to have a light version of D8 based on the same source file as the
current full version. The light version does not require anything from
v8::internal:: and can therefore be linked against shared v8 library.

Apparently this is necessary in some cases for the test shell according to Rico.
Until now this has been done by a light version of the sample shell, which we
are going to retire.

http://codereview.chromium.org/7351017/diff/6/src/d8.cc
File src/d8.cc (right):

http://codereview.chromium.org/7351017/diff/6/src/d8.cc#newcode768
src/d8.cc:768: FILE* file = fopen(name, "rb"); // TODO: reading from a directory
hangs!
This is a known minor issue. I'll come with a solution for this in another CL.

http://codereview.chromium.org/7351017/diff/6/src/d8.cc#newcode958
src/d8.cc:958: FILE* file = fopen(name, "rb"); // TODO: reading from a directory
hangs!
Ditto.

http://codereview.chromium.org/7351017/diff/6/src/d8.cc#newcode1052
src/d8.cc:1052: } else if (strcmp(argv[i], "--preemption") == 0) {
Many options are not supported by the light version.

[Søren Thygesen Gjesse, 2011-07-13 14:12:38]

LGTM with comments.

Is the long term plan to narrow down the difference between the full and light
version of d8?

It troubles me a bit that depending on whether shared libraries are used or not
the d8 binary have different capabilities.

http://codereview.chromium.org/7351017/diff/6/SConstruct
File SConstruct (right):

http://codereview.chromium.org/7351017/diff/6/SConstruct#newcode778
SConstruct:778: 'CPPDEFINES': ['USING_V8_SHARED'],
The define USING_V8_SHARED is for Windows. On the other platforms V8_SHARED
needs to be defined (see the comments at the top of include/v8.h). You can also
check with tools/gyp/v8.gyp.

http://codereview.chromium.org/7351017/diff/6/src/d8.cc
File src/d8.cc (right):

http://codereview.chromium.org/7351017/diff/6/src/d8.cc#newcode30
src/d8.cc:30: #define D8_LIGHT
How about renaming D8_LIGHT to D8_WITH_SHARED?

It would be even better if we could just use USING_V8_SHARED instead of adding a
new define.

http://codereview.chromium.org/7351017/diff/6/src/d8.cc#newcode44
src/d8.cc:44: #endif // D8_LIGHT
Two spaces before //. Also in other places below.

http://codereview.chromium.org/7351017/diff/6/src/d8.cc#newcode228
src/d8.cc:228: buffer[strlen(buffer) - 1] = '\0'; // chomp
Check for '\' before overwriting with '\0'?

Two spaces between ; and //.

Start comment with uppercase and end with dot - and maybe more elaborate.

http://codereview.chromium.org/7351017/diff/6/src/d8.cc#newcode261
src/d8.cc:261: static const int kMaxLength = 0x3fffffff;
Maybe assert that this is equal to  i::ExternalArray::kMaxLength (if not
building the light version).

http://codereview.chromium.org/7351017/diff/6/src/d8.cc#newcode479
src/d8.cc:479: counters_file_ = i::OS::MemoryMappedFile::create(name,
While at it consider moving name to the line with parameters.

http://codereview.chromium.org/7351017/diff/6/src/d8.cc#newcode842
src/d8.cc:842: printf("V8 version %s [D8 light with shared library]\n",
V8::GetVersion());
with -> using

http://codereview.chromium.org/7351017/diff/6/src/d8.cc#newcode1245
src/d8.cc:1245: #if defined(D8_FULL) && defined(ENABLE_DEBUGGER_SUPPORT)
D8_FULL is newer defined. Did you mean !defined(V8_LIGHT).

http://codereview.chromium.org/7351017/diff/6/src/d8.cc#newcode1281
src/d8.cc:1281: #undef D8_FULL
D8_FULL?

[Yang, 2011-07-13 15:42:25]

I replaced D8_LIGHT by USING_V8_SHARED.

http://codereview.chromium.org/7351017/diff/2010/src/d8.cc
File src/d8.cc (right):

http://codereview.chromium.org/7351017/diff/2010/src/d8.cc#newcode238
src/d8.cc:238: return accumulator;
I took advantage of String::Concat.
i::ReadLine(const char*) is now unused code btw.

[Yang, 2011-07-14 13:33:05]

Mads,
could you take over Soren's role as reviewer on this CL? There are just a few
changes I made upon Soren's suggestions.
- Yang

[Mads Ager (chromium), 2011-07-14 15:21:53]

OK, LGTM