Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(116)

Issue 3412034: Avoid logging preparse-data inside lazily compiled functions. (Closed)

Created:
10 years, 2 months ago by Lasse Reichstein
Modified:
9 years, 6 months ago
CC:
v8-dev
Visibility:
Public.

Description

Avoid logging preparse-data inside lazily compiled functions. Reduces size of preparser data significantly when there are nested functions. Also allows us to drop the "skip" fields of function entries, that tells us how much preparse-data to skip when skipping the function source.

Patch Set 1 #

Total comments: 14

Patch Set 2 : Addressed review comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+109 lines, -74 lines) Patch
M src/parser.h View 4 chunks +2 lines, -35 lines 0 comments Download
M src/parser.cc View 1 17 chunks +63 lines, -30 lines 0 comments Download
M src/scopes.h View 1 5 chunks +44 lines, -8 lines 0 comments Download
M src/scopes.cc View 1 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 3 (0 generated)
Lasse Reichstein
10 years, 2 months ago (2010-09-27 13:08:29 UTC) #1
Søren Thygesen Gjesse
LGTM http://codereview.chromium.org/3412034/diff/1/2 File src/parser.cc (right): http://codereview.chromium.org/3412034/diff/1/2#newcode996 src/parser.cc:996: virtual void ResumeRecording() { ASSERT(pause_count > 0? http://codereview.chromium.org/3412034/diff/1/2#newcode1376 ...
10 years, 2 months ago (2010-09-28 06:28:59 UTC) #2
Lasse Reichstein
10 years, 2 months ago (2010-09-28 07:17:06 UTC) #3
http://codereview.chromium.org/3412034/diff/1/2
File src/parser.cc (right):

http://codereview.chromium.org/3412034/diff/1/2#newcode996
src/parser.cc:996: virtual void ResumeRecording() {
On 2010/09/28 06:28:59, Søren Gjesse wrote:
> ASSERT(pause_count > 0?

Done.

http://codereview.chromium.org/3412034/diff/1/2#newcode1376
src/parser.cc:1376: parent->type_ = type;  // TODO(lrn): Remove if not needed.
Removed, it was just a mental note to check whether it's really needed.

http://codereview.chromium.org/3412034/diff/1/2#newcode1377
src/parser.cc:1377: // Initialize functiois n hijacked by DummyScope to
increment scope depth.
Drag'n'drop error? Should be "function is".

http://codereview.chromium.org/3412034/diff/1/4
File src/scopes.cc (right):

http://codereview.chromium.org/3412034/diff/1/4#newcode204
src/scopes.cc:204: void Scope::Leave() {
On 2010/09/28 06:28:59, Søren Gjesse wrote:
> This could be in the .h file.

Done.

http://codereview.chromium.org/3412034/diff/1/5
File src/scopes.h (right):

http://codereview.chromium.org/3412034/diff/1/5#newcode388
src/scopes.h:388: inside_with_level_(-1) {
On 2010/09/28 06:28:59, Søren Gjesse wrote:
> Shouldn't there be a constant for this value?

Done.

http://codereview.chromium.org/3412034/diff/1/5#newcode395
src/scopes.h:395: if (inside_with && inside_with_level_ < 0) {
On 2010/09/28 06:28:59, Søren Gjesse wrote:
> Compare with constant instead of < 0?

Done.

http://codereview.chromium.org/3412034/diff/1/5#newcode423
src/scopes.h:423: // Highest scope nesting level that is contained in a with
statement, or -1
On 2010/09/28 06:28:59, Søren Gjesse wrote:
> Highest -> Lowest/First?

Done.

Powered by Google App Engine
This is Rietveld 408576698