Fixing parallel execution in d8 (with -p) and some memory leaks.

Fixing parallel execution in d8 (with -p) and some memory leaks.

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

[Yang, 2011-09-13 10:57:10]

PTAL.

[Jakob Kummerow, 2011-09-13 11:55:11]

Looks pretty good, just a few comments.

http://codereview.chromium.org/7891005/diff/1/src/d8.cc
File src/d8.cc (right):

http://codereview.chromium.org/7891005/diff/1/src/d8.cc#newcode930
src/d8.cc:930: ShellThread(int no, char* files)
As discussed, let's add comments à la:
// Takes ownership of |files|.

http://codereview.chromium.org/7891005/diff/1/src/d8.cc#newcode968
src/d8.cc:968: break;
s/break/continue/

http://codereview.chromium.org/7891005/diff/1/src/d8.cc#newcode1189
src/d8.cc:1189: options.parallel_files[parallel_files_set++] = argv[++i];
I'd prefer to have the increment operations on their own lines, but since I
can't find that rule in the style guide I leave it up to you.

http://codereview.chromium.org/7891005/diff/1/src/d8.cc#newcode1226
src/d8.cc:1226: char* files;
We don't like uninitialized pointers. It's safe in this case, but "char* files =
NULL;" is better.

http://codereview.chromium.org/7891005/diff/1/src/d8.cc#newcode1239
src/d8.cc:1239: }
+1 :-)

http://codereview.chromium.org/7891005/diff/1/src/d8.cc#newcode1280
src/d8.cc:1280: && options.use_preemption) {
Both conditions could go in one line, I think.

[fschneider, 2011-09-13 12:08:57]

drive-by:

http://codereview.chromium.org/7891005/diff/1/src/d8.h
File src/d8.h (right):

http://codereview.chromium.org/7891005/diff/1/src/d8.h#newcode197
src/d8.h:197: if (parallel_files != NULL) delete[] parallel_files;
Same here.

http://codereview.chromium.org/7891005/diff/1/src/d8.h#newcode199
src/d8.h:199: if (isolate_sources != NULL) delete[] isolate_sources;
No need to check for NULL for delete.

[Yang, 2011-09-13 12:52:19]

Applied suggestions and switched to SmartArrayPointer in one occasion. Take
another look please.

[Jakob Kummerow, 2011-09-13 13:08:03]

LGTM if you address the comment below:

http://codereview.chromium.org/7891005/diff/4001/src/d8.cc
File src/d8.cc (right):

http://codereview.chromium.org/7891005/diff/4001/src/d8.cc#newcode1226
src/d8.cc:1226: i::SmartArrayPointer<char> files;
This will delete the contents of |files| when it goes out of scope at the end of
the loop. You should change back to plain pointers here (or use files.Detach()
in the method call below, but IMHO using plain pointers here is easier).