From: lrn@chromium.org Date: Fri, 27 Aug 2010 08:26:29 +0000 (+0000) Subject: Reordered function entries in PreParse data to be ordered by start position. X-Git-Tag: upstream/4.7.83~21280 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=7b89a5f2b85f0bf8cc7227ea0877624c3c04fca3;p=platform%2Fupstream%2Fv8.git Reordered function entries in PreParse data to be ordered by start position. Also add skip to entry, to skip pre-data for the body of the function. Preparser data is now only accessed linearly, in the same order it was created. Review URL: http://codereview.chromium.org/3185026 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@5361 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/parser.cc b/src/parser.cc index 2bf73c4..f0b6f35 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -867,11 +867,10 @@ class ParserLog BASE_EMBEDDED { public: virtual ~ParserLog() { } - // Records the occurrence of a function. The returned object is - // only guaranteed to be valid until the next function has been - // logged. + // Records the occurrence of a function. virtual FunctionEntry LogFunction(int start) { return FunctionEntry(); } - + // Return the current position in the function entry log. + virtual int position() { return 0; } virtual void LogError() { } }; @@ -913,30 +912,45 @@ class ParserRecorder: public ParserLog { const char* message, Vector args); Vector ExtractData() { - return store_.ToVector(); + int total_size = ScriptDataImpl::kHeaderSize + store_.size(); + Vector data = Vector::New(total_size); + memcpy(data.start(), preamble_, sizeof(preamble_)); + if (ScriptDataImpl::kHeaderSize < total_size) { + store_.WriteTo(data.SubVector(ScriptDataImpl::kHeaderSize, total_size)); + } + return data; } + virtual int position() { return store_.size(); } private: - bool has_error_; Collector store_; - Vector preamble_; + unsigned preamble_[ScriptDataImpl::kHeaderSize]; +#ifdef DEBUG + int prev_start; +#endif - Collector* store() { return &store_; } + bool has_error() { + return static_cast(preamble_[ScriptDataImpl::kHasErrorOffset]); + } void WriteString(Vector str); }; -FunctionEntry ScriptDataImpl::GetFunctionEnd(int start) { - if (nth(last_entry_).start_pos() > start) { - // If the last entry we looked up is higher than what we're - // looking for then it's useless and we reset it. - last_entry_ = 0; - } - for (int i = last_entry_; i < EntryCount(); i++) { - FunctionEntry entry = nth(i); - if (entry.start_pos() == start) { - last_entry_ = i; - return entry; - } +void ScriptDataImpl::SkipFunctionEntry(int start) { + ASSERT(index_ + FunctionEntry::kSize <= store_.length()); + ASSERT(static_cast(store_[index_]) == start); + index_ += FunctionEntry::kSize; +} + + +FunctionEntry ScriptDataImpl::GetFunctionEntry(int start) { + // The current pre-data entry must be a FunctionEntry with the given + // start position. + if ((index_ + FunctionEntry::kSize <= store_.length()) + && (static_cast(store_[index_]) == start)) { + int index = index_; + index_ += FunctionEntry::kSize; + return FunctionEntry(store_.SubVector(index, + index + FunctionEntry::kSize)); } return FunctionEntry(); } @@ -952,31 +966,23 @@ bool ScriptDataImpl::SanityCheck() { } -int ScriptDataImpl::EntryCount() { - return (store_.length() - kHeaderSize) / FunctionEntry::kSize; -} - - -FunctionEntry ScriptDataImpl::nth(int n) { - int offset = kHeaderSize + n * FunctionEntry::kSize; - return FunctionEntry(Vector(store_.start() + offset, - FunctionEntry::kSize)); -} - - ParserRecorder::ParserRecorder() - : has_error_(false), store_(ScriptDataImpl::kHeaderSize) { - preamble_ = store()->AddBlock(ScriptDataImpl::kHeaderSize, 0); + : store_(0) { +#ifdef DEBUG + prev_start = -1; +#endif preamble_[ScriptDataImpl::kMagicOffset] = ScriptDataImpl::kMagicNumber; preamble_[ScriptDataImpl::kVersionOffset] = ScriptDataImpl::kCurrentVersion; preamble_[ScriptDataImpl::kHasErrorOffset] = false; + preamble_[ScriptDataImpl::kSizeOffset] = 0; + ASSERT_EQ(4, ScriptDataImpl::kHeaderSize); } void ParserRecorder::WriteString(Vector str) { - store()->Add(str.length()); + store_.Add(str.length()); for (int i = 0; i < str.length(); i++) { - store()->Add(str[i]); + store_.Add(str[i]); } } @@ -995,15 +1001,12 @@ const char* ScriptDataImpl::ReadString(unsigned* start, int* chars) { void ParserRecorder::LogMessage(Scanner::Location loc, const char* message, Vector args) { - if (has_error_) return; - store()->Reset(); - preamble_ = store()->AddBlock(ScriptDataImpl::kHeaderSize, 0); - preamble_[ScriptDataImpl::kMagicOffset] = ScriptDataImpl::kMagicNumber; - preamble_[ScriptDataImpl::kVersionOffset] = ScriptDataImpl::kCurrentVersion; + if (has_error()) return; preamble_[ScriptDataImpl::kHasErrorOffset] = true; - store()->Add(loc.beg_pos); - store()->Add(loc.end_pos); - store()->Add(args.length()); + store_.Reset(); + store_.Add(loc.beg_pos); + store_.Add(loc.end_pos); + store_.Add(args.length()); WriteString(CStrVector(message)); for (int i = 0; i < args.length(); i++) { WriteString(CStrVector(args[i])); @@ -1046,10 +1049,22 @@ unsigned* ScriptDataImpl::ReadAddress(int position) { return &store_[ScriptDataImpl::kHeaderSize + position]; } +void ScriptDataImpl::FindStart(int position) { + // Only search forwards, and linearly for now. + while ((index_ < store_.length()) + && (static_cast(store_[index_])) < position) { + index_ += FunctionEntry::kSize; + } +} + FunctionEntry ParserRecorder::LogFunction(int start) { - if (has_error_) return FunctionEntry(); - FunctionEntry result(store()->AddBlock(FunctionEntry::kSize, 0)); +#ifdef DEBUG + ASSERT(start > prev_start); + prev_start = start; +#endif + if (has_error()) return FunctionEntry(); + FunctionEntry result(store_.AddBlock(FunctionEntry::kSize, 0)); result.set_start_pos(start); return result; } @@ -3939,43 +3954,52 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle var_name, bool is_lazily_compiled = mode() == PARSE_LAZILY && top_scope_->HasTrivialOuterContext(); + int function_block_pos = scanner_.location().beg_pos; int materialized_literal_count; int expected_property_count; + int end_pos; bool only_simple_this_property_assignments; Handle this_property_assignments; if (is_lazily_compiled && pre_data() != NULL) { - FunctionEntry entry = pre_data()->GetFunctionEnd(start_pos); + FunctionEntry entry = pre_data()->GetFunctionEntry(function_block_pos); if (!entry.is_valid()) { ReportInvalidPreparseData(name, CHECK_OK); } - int end_pos = entry.end_pos(); - if (end_pos <= start_pos) { + end_pos = entry.end_pos(); + if (end_pos <= function_block_pos) { // End position greater than end of stream is safe, and hard to check. ReportInvalidPreparseData(name, CHECK_OK); } - Counters::total_preparse_skipped.Increment(end_pos - start_pos); + Counters::total_preparse_skipped.Increment(end_pos - function_block_pos); scanner_.SeekForward(end_pos); + pre_data()->Skip(entry.predata_skip()); materialized_literal_count = entry.literal_count(); expected_property_count = entry.property_count(); only_simple_this_property_assignments = false; this_property_assignments = Factory::empty_fixed_array(); + Expect(Token::RBRACE, CHECK_OK); } else { + if (pre_data() != NULL) { + // Skip pre-data entry for non-lazily compiled function. + pre_data()->SkipFunctionEntry(function_block_pos); + } + FunctionEntry entry = log()->LogFunction(function_block_pos); + int predata_position_before = log()->position(); ParseSourceElements(&body, Token::RBRACE, CHECK_OK); materialized_literal_count = temp_scope.materialized_literal_count(); expected_property_count = temp_scope.expected_property_count(); only_simple_this_property_assignments = temp_scope.only_simple_this_property_assignments(); this_property_assignments = temp_scope.this_property_assignments(); - } - - Expect(Token::RBRACE, CHECK_OK); - int end_pos = scanner_.location().end_pos; - FunctionEntry entry = log()->LogFunction(start_pos); - if (entry.is_valid()) { - entry.set_end_pos(end_pos); - entry.set_literal_count(materialized_literal_count); - entry.set_property_count(expected_property_count); + Expect(Token::RBRACE, CHECK_OK); + end_pos = scanner_.location().end_pos; + if (entry.is_valid()) { + entry.set_end_pos(end_pos); + entry.set_literal_count(materialized_literal_count); + entry.set_property_count(expected_property_count); + entry.set_predata_skip(log()->position() - predata_position_before); + } } FunctionLiteral* function_literal = diff --git a/src/parser.h b/src/parser.h index 225a368..2952581 100644 --- a/src/parser.h +++ b/src/parser.h @@ -68,11 +68,18 @@ class FunctionEntry BASE_EMBEDDED { void set_literal_count(int value) { backing_[kLiteralCountOffset] = value; } int property_count() { return backing_[kPropertyCountOffset]; } - void set_property_count(int value) { backing_[kPropertyCountOffset] = value; } + void set_property_count(int value) { + backing_[kPropertyCountOffset] = value; + } + + int predata_skip() { return backing_[kPredataSkipOffset]; } + void set_predata_skip(int value) { + backing_[kPredataSkipOffset] = value; + } bool is_valid() { return backing_.length() > 0; } - static const int kSize = 4; + static const int kSize = 5; private: Vector backing_; @@ -80,6 +87,7 @@ class FunctionEntry BASE_EMBEDDED { static const int kEndPosOffset = 1; static const int kLiteralCountOffset = 2; static const int kPropertyCountOffset = 3; + static const int kPredataSkipOffset = 4; }; @@ -87,12 +95,13 @@ class ScriptDataImpl : public ScriptData { public: explicit ScriptDataImpl(Vector store) : store_(store), - last_entry_(0) { } + index_(kHeaderSize) { } virtual ~ScriptDataImpl(); virtual int Length(); virtual const char* Data(); virtual bool HasError(); - FunctionEntry GetFunctionEnd(int start); + FunctionEntry GetFunctionEntry(int start); + void SkipFunctionEntry(int start); bool SanityCheck(); Scanner::Location MessageLocation(); @@ -102,31 +111,33 @@ class ScriptDataImpl : public ScriptData { bool has_error() { return store_[kHasErrorOffset]; } unsigned magic() { return store_[kMagicOffset]; } unsigned version() { return store_[kVersionOffset]; } + // Skip forward in the preparser data by the given number + // of unsigned ints. + virtual void Skip(int entries) { + ASSERT(entries >= 0); + ASSERT(entries <= store_.length() - index_); + index_ += entries; + } static const unsigned kMagicNumber = 0xBadDead; static const unsigned kCurrentVersion = 1; - static const unsigned kMagicOffset = 0; - static const unsigned kVersionOffset = 1; - static const unsigned kHasErrorOffset = 2; - static const unsigned kSizeOffset = 3; - static const unsigned kHeaderSize = 4; + static const int kMagicOffset = 0; + static const int kVersionOffset = 1; + static const int kHasErrorOffset = 2; + static const int kSizeOffset = 3; + static const int kHeaderSize = 4; private: + Vector store_; + int index_; + unsigned Read(int position); unsigned* ReadAddress(int position); - int EntryCount(); - FunctionEntry nth(int n); - - Vector store_; + void FindStart(int position); // Read strings written by ParserRecorder::WriteString. static const char* ReadString(unsigned* start, int* chars); - - // The last entry returned. This is used to make lookup faster: - // the next entry to return is typically the next entry so lookup - // will usually be much faster if we start from the last entry. - int last_entry_; }; diff --git a/src/scanner.h b/src/scanner.h index 53aaad6..8d61846 100644 --- a/src/scanner.h +++ b/src/scanner.h @@ -77,7 +77,7 @@ class UTF8Buffer { static const char kEndMarker = '\x00'; private: static const int kInitialCapacity = 256; - SequenceCollector buffer_; + SequenceCollector buffer_; void AddCharSlow(uc32 c); }; diff --git a/src/utils.h b/src/utils.h index 90fa74f..d605891 100644 --- a/src/utils.h +++ b/src/utils.h @@ -326,9 +326,9 @@ class Vector { // Returns a vector using the same backing storage as this one, // spanning from and including 'from', to but not including 'to'. Vector SubVector(int from, int to) { - ASSERT(from < length_); ASSERT(to <= length_); ASSERT(from < to); + ASSERT(0 <= from); return Vector(start() + from, to - from); } @@ -485,24 +485,20 @@ inline Vector< Handle > HandleVector(v8::internal::Handle* elms, * in memory). The collector may move elements unless it has guaranteed not * to. */ -template +template class Collector { public: - Collector(int initial_capacity = kMinCapacity, - int growth_factor = 2, - int max_growth = 1 * MB) - : growth_factor_(growth_factor), max_growth_(max_growth) { + explicit Collector(int initial_capacity = kMinCapacity) + : index_(0), size_(0) { if (initial_capacity < kMinCapacity) { initial_capacity = kMinCapacity; } - current_chunk_ = NewArray(initial_capacity); - current_capacity_ = initial_capacity; - index_ = 0; + current_chunk_ = Vector::New(initial_capacity); } virtual ~Collector() { // Free backing store (in reverse allocation order). - DeleteArray(current_chunk_); + current_chunk_.Dispose(); for (int i = chunks_.length() - 1; i >= 0; i--) { chunks_.at(i).Dispose(); } @@ -510,11 +506,12 @@ class Collector { // Add a single element. inline void Add(T value) { - if (index_ >= current_capacity_) { + if (index_ >= current_chunk_.length()) { Grow(1); } current_chunk_[index_] = value; index_++; + size_++; } // Add a block of contiguous elements and return a Vector backed by the @@ -522,11 +519,13 @@ class Collector { // A basic Collector will keep this vector valid as long as the Collector // is alive. inline Vector AddBlock(int size, T initial_value) { - if (index_ + size > current_capacity_) { + ASSERT(size > 0); + if (size > current_chunk_.length() - index_) { Grow(size); } - T* position = current_chunk_ + index_; + T* position = current_chunk_.start() + index_; index_ += size; + size_ += size; for (int i = 0; i < size; i++) { position[i] = initial_value; } @@ -534,30 +533,31 @@ class Collector { } - // Allocate a single contiguous vector, copy all the collected - // elements to the vector, and return it. - // The caller is responsible for freeing the memory of the returned - // vector (e.g., using Vector::Dispose). - Vector ToVector() { - // Find the total length. - int total_length = index_; - for (int i = 0; i < chunks_.length(); i++) { - total_length += chunks_.at(i).length(); - } - T* new_store = NewArray(total_length); + // Write the contents of the collector into the provided vector. + void WriteTo(Vector destination) { + ASSERT(size_ <= destination.length()); int position = 0; for (int i = 0; i < chunks_.length(); i++) { Vector chunk = chunks_.at(i); for (int j = 0; j < chunk.length(); j++) { - new_store[position] = chunk[j]; + destination[position] = chunk[j]; position++; } } for (int i = 0; i < index_; i++) { - new_store[position] = current_chunk_[i]; + destination[position] = current_chunk_[i]; position++; } - return Vector(new_store, total_length); + } + + // Allocate a single contiguous vector, copy all the collected + // elements to the vector, and return it. + // The caller is responsible for freeing the memory of the returned + // vector (e.g., using Vector::Dispose). + Vector ToVector() { + Vector new_store = Vector::New(size_); + WriteTo(new_store); + return new_store; } // Resets the collector to be empty. @@ -567,35 +567,42 @@ class Collector { } chunks_.Rewind(0); index_ = 0; + size_ = 0; } + // Total number of elements added to collector so far. + inline int size() { return size_; } + protected: static const int kMinCapacity = 16; List > chunks_; - T* current_chunk_; - int growth_factor_; - int max_growth_; - int current_capacity_; - int index_; + Vector current_chunk_; // Block of memory currently being written into. + int index_; // Current index in current chunk. + int size_; // Total number of elements in collector. // Creates a new current chunk, and stores the old chunk in the chunks_ list. void Grow(int min_capacity) { - ASSERT(growth_factor_ > 1); - int growth = current_capacity_ * (growth_factor_ - 1); - if (growth > max_growth_) { - growth = max_growth_; + ASSERT(growth_factor > 1); + int growth = current_chunk_.length() * (growth_factor - 1); + if (growth > max_growth) { + growth = max_growth; } - int new_capacity = current_capacity_ + growth; + int new_capacity = current_chunk_.length() + growth; if (new_capacity < min_capacity) { - new_capacity = min_capacity; + new_capacity = min_capacity + growth; + } + Vector new_chunk = Vector::New(new_capacity); + int new_index = PrepareGrow(new_chunk); + if (index_ > 0) { + chunks_.Add(current_chunk_.SubVector(0, index_)); + } else { + // Can happen if the call to PrepareGrow moves everything into + // the new chunk. + current_chunk_.Dispose(); } - T* new_chunk = NewArray(new_capacity); - int new_index = PrepareGrow(Vector(new_chunk, new_capacity)); - chunks_.Add(Vector(current_chunk_, index_)); current_chunk_ = new_chunk; - current_capacity_ = new_capacity; index_ = new_index; - ASSERT(index_ + min_capacity <= current_capacity_); + ASSERT(index_ + min_capacity <= current_chunk_.length()); } // Before replacing the current chunk, give a subclass the option to move @@ -617,13 +624,11 @@ class Collector { * NOTICE: Blocks allocated using Collector::AddBlock(int) can move * as well, if inside an active sequence where another element is added. */ -template -class SequenceCollector : public Collector { +template +class SequenceCollector : public Collector { public: - SequenceCollector(int initial_capacity, - int growth_factor = 2, - int max_growth = 1 * MB) - : Collector(initial_capacity, growth_factor, max_growth), + explicit SequenceCollector(int initial_capacity) + : Collector(initial_capacity), sequence_start_(kNoSequence) { } virtual ~SequenceCollector() {} @@ -637,20 +642,22 @@ class SequenceCollector : public Collector { ASSERT(sequence_start_ != kNoSequence); int sequence_start = sequence_start_; sequence_start_ = kNoSequence; - return Vector(this->current_chunk_ + sequence_start, - this->index_ - sequence_start); + if (sequence_start == this->index_) return Vector(); + return this->current_chunk_.SubVector(sequence_start, this->index_); } // Drops the currently added sequence, and all collected elements in it. void DropSequence() { ASSERT(sequence_start_ != kNoSequence); + int sequence_length = this->index_ - sequence_start_; this->index_ = sequence_start_; + this->size_ -= sequence_length; sequence_start_ = kNoSequence; } virtual void Reset() { sequence_start_ = kNoSequence; - this->Collector::Reset(); + this->Collector::Reset(); } private: diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index adaf102..4b6fa9c 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -8603,7 +8603,7 @@ TEST(PreCompileInvalidPreparseDataError) { // ScriptDataImpl private implementation details const int kUnsignedSize = sizeof(unsigned); const int kHeaderSize = 4; - const int kFunctionEntrySize = 4; + const int kFunctionEntrySize = 5; const int kFunctionEntryStartOffset = 0; const int kFunctionEntryEndOffset = 1; unsigned* sd_data = @@ -8625,6 +8625,8 @@ TEST(PreCompileInvalidPreparseDataError) { try_catch.Reset(); // Overwrite function bar's start position with 200. The function entry // will not be found when searching for it by position. + sd = v8::ScriptData::PreCompile(script, i::StrLength(script)); + sd_data = reinterpret_cast(const_cast(sd->Data())); sd_data[kHeaderSize + 1 * kFunctionEntrySize + kFunctionEntryStartOffset] = 200; compiled_script = Script::New(source, NULL, sd);