Remove support for logging into a memory buffer.

Remove support for logging into a memory buffer.

The only usage of it was in logging tests, I've switched them for
using a file.

I've left out support for "--logfile=*" for now, as Chromium uses it.
Will be removed after the next V8 roll.

R=sgjesse@chromium.org
BUG=859
TEST=mjsunit/log-*

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

[Søren Thygesen Gjesse, 2011-07-07 11:59:53]

http://codereview.chromium.org/7310025/diff/1/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/7310025/diff/1/src/runtime.cc#newcode12160
src/runtime.cc:12160: 
I don't think that specialized testing functions like CreateClassWithCallback
and CreateClassWithGetterAndSetter realy belong in runtime.cc. Please rewrite
the tests using these as cctests.

http://codereview.chromium.org/7310025/diff/1/src/runtime.cc#newcode12217
src/runtime.cc:12217: RUNTIME_FUNCTION(MaybeObject*, Runtime_CollectGarbage2) {
I don't see any difference between this and the existing Runtime_CollectGarbage.

http://codereview.chromium.org/7310025/diff/1/test/mjsunit/log-eq-of-logging-...
File test/mjsunit/log-eq-of-logging-and-traversal.js (right):

http://codereview.chromium.org/7310025/diff/1/test/mjsunit/log-eq-of-logging-...
test/mjsunit/log-eq-of-logging-and-traversal.js:35: 
Please add a comment here to explain that first the normal logging is used (as
profiling is turned on) and then the traversal one normally used when starting
logging is used.

http://codereview.chromium.org/7310025/diff/1/test/mjsunit/log-eq-of-logging-...
test/mjsunit/log-eq-of-logging-and-traversal.js:140: if (!entityA && entityB)
return true;
!entityA -> entityA == null?

http://codereview.chromium.org/7310025/diff/1/test/mjsunit/log-eq-of-logging-...
test/mjsunit/log-eq-of-logging-and-traversal.js:141: if (entityA && !entityB)
return false;
Ditto.

http://codereview.chromium.org/7310025/diff/1/test/mjsunit/log-eq-of-logging-...
test/mjsunit/log-eq-of-logging-and-traversal.js:147: var equal = true;
Please explain how this comparison works, and why there are more data in
logging_entries.

[mnaganov (inactive), 2011-07-07 20:19:57]

Thanks for comments. I disagree with one of them, please look.

http://codereview.chromium.org/7310025/diff/1/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/7310025/diff/1/src/runtime.cc#newcode12160
src/runtime.cc:12160: 
On 2011/07/07 11:59:53, Søren Gjesse wrote:
> I don't think that specialized testing functions like CreateClassWithCallback
> and CreateClassWithGetterAndSetter realy belong in runtime.cc. Please rewrite
> the tests using these as cctests.

Those tests were cctests initially. Two reasons I've converted them into mjsunit
tests (and moved utility code into runtime): 1) parsing logs in JS is much
easier that in C++ code; 2) I already have implemented support for passing logs
data into mjsunit tests, so I thought it would be a good idea to reuse it for
these two tests.

Perhaps, we can generalize them to make more useful for runtime? Something
similar to CreateApiFunction? What do you think?

http://codereview.chromium.org/7310025/diff/1/src/runtime.cc#newcode12217
src/runtime.cc:12217: RUNTIME_FUNCTION(MaybeObject*, Runtime_CollectGarbage2) {
On 2011/07/07 11:59:53, Søren Gjesse wrote:
> I don't see any difference between this and the existing
Runtime_CollectGarbage.

My bad. I was receiving "illegal access" error, and wrongly attributed it to the
fact that the function is declared in "debugger support" list, and thus requires
to be called from some special context. But then I found that it simply requires
one argument which it doesn't actually use.

Removed.

http://codereview.chromium.org/7310025/diff/1/test/mjsunit/log-eq-of-logging-...
File test/mjsunit/log-eq-of-logging-and-traversal.js (right):

http://codereview.chromium.org/7310025/diff/1/test/mjsunit/log-eq-of-logging-...
test/mjsunit/log-eq-of-logging-and-traversal.js:35: 
On 2011/07/07 11:59:53, Søren Gjesse wrote:
> Please add a comment here to explain that first the normal logging is used (as
> profiling is turned on) and then the traversal one normally used when starting
> logging is used.

Done.

http://codereview.chromium.org/7310025/diff/1/test/mjsunit/log-eq-of-logging-...
test/mjsunit/log-eq-of-logging-and-traversal.js:140: if (!entityA && entityB)
return true;
On 2011/07/07 11:59:53, Søren Gjesse wrote:
> !entityA -> entityA == null?

Done.

http://codereview.chromium.org/7310025/diff/1/test/mjsunit/log-eq-of-logging-...
test/mjsunit/log-eq-of-logging-and-traversal.js:141: if (entityA && !entityB)
return false;
On 2011/07/07 11:59:53, Søren Gjesse wrote:
> Ditto.

Done.

http://codereview.chromium.org/7310025/diff/1/test/mjsunit/log-eq-of-logging-...
test/mjsunit/log-eq-of-logging-and-traversal.js:147: var equal = true;
On 2011/07/07 11:59:53, Søren Gjesse wrote:
> Please explain how this comparison works, and why there are more data in
> logging_entries.

Done.

[mnaganov (inactive), 2011-07-08 09:56:37]

Daniel, what do you think about having 2 small test-only runtime functions? The
only feasible alternative I see is using a cctest that will use a temporary log
file, but this means we will need to implement support for them in V8, while we
already have it in Python. And also we need more lines of code for parsing log
files in C++ vs. JS.

[danno, 2011-07-08 12:32:10]

I agree with Søren, please avoid adding anything to runtime.cc that is only
there for testing. Even in a cctest, there's no reason that you can't execute a
substantial chunk of js code as part of the test, either as a static string or a
file that you load from an external source, so I think you can pretty much keep
most of the code still in js rather than C++. Would this solve the problem of
allowing you to do most of the heavy log-parsing lifting in js?

[mnaganov (inactive), 2011-07-13 10:04:34]

On 2011/07/08 12:32:10, danno wrote:
> I agree with Søren, please avoid adding anything to runtime.cc that is only
> there for testing. Even in a cctest, there's no reason that you can't execute
a
> substantial chunk of js code as part of the test, either as a static string or
a
> file that you load from an external source, so I think you can pretty much
keep
> most of the code still in js rather than C++. Would this solve the problem of
> allowing you to do most of the heavy log-parsing lifting in js?

I've changed my approach to avoid adding runtime functions used only in tests.
Please re-review.
Sorry for a big patch, but the bigger part of the change consists code removals.

[Søren Thygesen Gjesse, 2011-07-13 10:51:39]

LGTM

[Sven Panne, 2011-07-13 13:11:51]

http://codereview.chromium.org/7310025/diff/13001/test/cctest/test-log.cc
File test/cctest/test-log.cc (right):

http://codereview.chromium.org/7310025/diff/13001/test/cctest/test-log.cc#new...
test/cctest/test-log.cc:511: "tools/logreader.js",
"test/cctest/log-eq-of-logging-and-traversal.js"
This change broke testing in an architecture-specific subdirectory (e.g. cd
v8/build-ia32 && ../tools/test.py), please restore the previous behaviour.

[mnaganov (inactive), 2011-07-13 13:14:01]

On 2011/07/13 13:11:51, Sven wrote:
> http://codereview.chromium.org/7310025/diff/13001/test/cctest/test-log.cc
> File test/cctest/test-log.cc (right):
> 
>
http://codereview.chromium.org/7310025/diff/13001/test/cctest/test-log.cc#new...
> test/cctest/test-log.cc:511: "tools/logreader.js",
> "test/cctest/log-eq-of-logging-and-traversal.js"
> This change broke testing in an architecture-specific subdirectory (e.g. cd
> v8/build-ia32 && ../tools/test.py), please restore the previous behaviour.

Sorry, will fix now.