[reland] [parser] Skipping inner funcs: Use less memory for variables.

- Make it possible to store quarter-bytes instead of full bytes.

- Don't store is_used; it can be recovered correctly based on the actual full
  parse (when a lazy function is eventually called) and
  has_forced_scope_allocation.

- With the is_used change, the old testing approach (which compared a scope for
  which we didn't do scope allocation to the baseline) no longer made
  sense. Replaced it with a new testing approach, which is also closer to the
  actual usage.

- First version (reverted): https://chromium-review.googlesource.com/725422

BUG=v8:5516

Change-Id: I1468af6670b689a104bd867377caa1d236070820
Reviewed-on: https://chromium-review.googlesource.com/733123Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48903}

src/parsing/preparsed-scope-data.cc

@@ -20,17 +20,16 @@ class ScopeCallsSloppyEvalField : public BitField<bool, 0, 1> {};
 class InnerScopeCallsEvalField
     : public BitField<bool, ScopeCallsSloppyEvalField::kNext, 1> {};
 
-class VariableIsUsedField : public BitField16<bool, 0, 1> {};
-class VariableMaybeAssignedField
-    : public BitField16<bool, VariableIsUsedField::kNext, 1> {};
+class VariableMaybeAssignedField : public BitField8<bool, 0, 1> {};
 class VariableContextAllocatedField
-    : public BitField16<bool, VariableMaybeAssignedField::kNext, 1> {};
+    : public BitField8<bool, VariableMaybeAssignedField::kNext, 1> {};
 
 const int kMagicValue = 0xc0de0de;
 
 #ifdef DEBUG
 const size_t kUint32Size = 5;
 const size_t kUint8Size = 2;
+const size_t kQuarterMarker = 0;
 #else
 const size_t kUint32Size = 4;
 const size_t kUint8Size = 1;
@@ -39,8 +38,8 @@ const size_t kUint8Size = 1;
 const int kPlaceholderSize = kUint32Size;
 const int kSkippableFunctionDataSize = 4 * kUint32Size + 1 * kUint8Size;
 
-class LanguageField : public BitField<LanguageMode, 0, 1> {};
-class UsesSuperField : public BitField<bool, LanguageField::kNext, 1> {};
+class LanguageField : public BitField8<LanguageMode, 0, 1> {};
+class UsesSuperField : public BitField8<bool, LanguageField::kNext, 1> {};
 STATIC_ASSERT(LanguageModeSize <= LanguageField::kNumValues);
 
 }  // namespace
@@ -99,6 +98,7 @@ void ProducedPreParsedScopeData::ByteData::WriteUint32(uint32_t data) {
   for (int i = 0; i < 4; ++i) {
     backing_store_.push_back(*d++);
   }
+  free_quarters_in_last_byte_ = 0;
 }
 
 void ProducedPreParsedScopeData::ByteData::OverwriteFirstUint32(uint32_t data) {
@@ -121,6 +121,25 @@ void ProducedPreParsedScopeData::ByteData::WriteUint8(uint8_t data) {
   backing_store_.push_back(kUint8Size);
 #endif
   backing_store_.push_back(data);
+  free_quarters_in_last_byte_ = 0;
+}
+
+void ProducedPreParsedScopeData::ByteData::WriteQuarter(uint8_t data) {
+  DCHECK_LE(data, 3);
+  if (free_quarters_in_last_byte_ == 0) {
+#ifdef DEBUG
+    // Save a marker in debug mode.
+    backing_store_.push_back(kQuarterMarker);
+#endif
+    backing_store_.push_back(0);
+    free_quarters_in_last_byte_ = 3;
+  } else {
+    --free_quarters_in_last_byte_;
+  }
+
+  uint8_t shift_amount = free_quarters_in_last_byte_ * 2;
+  DCHECK_EQ(backing_store_.back() & (3 << shift_amount), 0);
+  backing_store_.back() |= (data << shift_amount);
 }
 
 Handle<PodArray<uint8_t>> ProducedPreParsedScopeData::ByteData::Serialize(
@@ -216,7 +235,7 @@ void ProducedPreParsedScopeData::AddSkippableFunction(
   uint8_t language_and_super = LanguageField::encode(language_mode) |
                                UsesSuperField::encode(uses_super_property);
 
-  byte_data_->WriteUint8(language_and_super);
+  byte_data_->WriteQuarter(language_and_super);
 }
 
 void ProducedPreParsedScopeData::SaveScopeAllocationData(
@@ -382,14 +401,11 @@ void ProducedPreParsedScopeData::SaveDataForVariable(Variable* var) {
     byte_data_->WriteUint8(name->raw_data()[i]);
   }
 #endif
-  // FIXME(marja): Only 3 bits needed, not a full byte.
-  byte variable_data = VariableIsUsedField::encode(var->is_used()) |
-                       VariableMaybeAssignedField::encode(
+  byte variable_data = VariableMaybeAssignedField::encode(
                            var->maybe_assigned() == kMaybeAssigned) |
                        VariableContextAllocatedField::encode(
                            var->has_forced_context_allocation());
-
-  byte_data_->WriteUint8(variable_data);
+  byte_data_->WriteQuarter(variable_data);
 }
 
 void ProducedPreParsedScopeData::SaveDataForInnerScopes(Scope* scope) {
@@ -429,6 +445,7 @@ int32_t ConsumedPreParsedScopeData::ByteData::ReadUint32() {
   for (int i = 0; i < 4; ++i) {
     *p++ = data_->get(index_++);
   }
+  stored_quarters_ = 0;
   return result;
 }
 
@@ -439,9 +456,29 @@ uint8_t ConsumedPreParsedScopeData::ByteData::ReadUint8() {
   // Check that there indeed is a byte following.
   DCHECK_EQ(data_->get(index_++), kUint8Size);
 #endif
+  stored_quarters_ = 0;
   return data_->get(index_++);
 }
 
+uint8_t ConsumedPreParsedScopeData::ByteData::ReadQuarter() {
+  DCHECK_NOT_NULL(data_);
+  if (stored_quarters_ == 0) {
+    DCHECK_GE(RemainingBytes(), kUint8Size);
+#ifdef DEBUG
+    // Check that there indeed are quarters following.
+    DCHECK_EQ(data_->get(index_++), kQuarterMarker);
+#endif
+    stored_byte_ = data_->get(index_++);
+    stored_quarters_ = 4;
+  }
+  // Read the first 2 bits from stored_byte_.
+  uint8_t result = (stored_byte_ >> 6) & 3;
+  DCHECK_LE(result, 3);
+  --stored_quarters_;
+  stored_byte_ <<= 2;
+  return result;
+}
+
 ConsumedPreParsedScopeData::ConsumedPreParsedScopeData()
     : scope_data_(new ByteData()), child_index_(0) {}
 
@@ -477,7 +514,7 @@ ConsumedPreParsedScopeData::GetDataForSkippableFunction(
   *num_parameters = scope_data_->ReadUint32();
   *num_inner_functions = scope_data_->ReadUint32();
 
-  uint8_t language_and_super = scope_data_->ReadUint8();
+  uint8_t language_and_super = scope_data_->ReadQuarter();
   *language_mode = LanguageMode(LanguageField::decode(language_and_super));
   *uses_super_property = UsesSuperField::decode(language_and_super);
 
@@ -518,13 +555,6 @@ void ConsumedPreParsedScopeData::RestoreScopeAllocationData(
   DCHECK_EQ(scope_data_->RemainingBytes(), 0);
 }
 
-void ConsumedPreParsedScopeData::SkipFunctionDataForTesting() {
-  ByteData::ReadingScope reading_scope(this);
-  scope_data_->SetPosition(0);
-  uint32_t scope_data_start = scope_data_->ReadUint32();
-  scope_data_->SetPosition(scope_data_start);
-}
-
 void ConsumedPreParsedScopeData::RestoreData(Scope* scope) {
   if (scope->is_declaration_scope() &&
       scope->AsDeclarationScope()->is_skipped_function()) {
@@ -581,15 +611,12 @@ void ConsumedPreParsedScopeData::RestoreDataForVariable(Variable* var) {
     DCHECK_EQ(scope_data_->ReadUint8(), name->raw_data()[i]);
   }
 #endif
-  CHECK_GE(scope_data_->RemainingBytes(), kUint8Size);
-  uint8_t variable_data = scope_data_->ReadUint8();
-  if (VariableIsUsedField::decode(variable_data)) {
-    var->set_is_used();
-  }
+  uint8_t variable_data = scope_data_->ReadQuarter();
   if (VariableMaybeAssignedField::decode(variable_data)) {
     var->set_maybe_assigned();
   }
   if (VariableContextAllocatedField::decode(variable_data)) {
+    var->set_is_used();
     var->ForceContextAllocation();
   }
 }

src/parsing/preparsed-scope-data.h

@@ -69,10 +69,12 @@ class ProducedPreParsedScopeData : public ZoneObject {
  public:
   class ByteData : public ZoneObject {
    public:
-    explicit ByteData(Zone* zone) : backing_store_(zone) {}
+    explicit ByteData(Zone* zone)
+        : backing_store_(zone), free_quarters_in_last_byte_(0) {}
 
     void WriteUint32(uint32_t data);
     void WriteUint8(uint8_t data);
+    void WriteQuarter(uint8_t data);
 
     // For overwriting previously written data at position 0.
     void OverwriteFirstUint32(uint32_t data);
@@ -83,6 +85,7 @@ class ProducedPreParsedScopeData : public ZoneObject {
 
    private:
     ZoneDeque<uint8_t> backing_store_;
+    uint8_t free_quarters_in_last_byte_;
   };
 
   // Create a ProducedPreParsedScopeData object which will collect data as we
@@ -181,7 +184,8 @@ class ConsumedPreParsedScopeData {
  public:
   class ByteData {
    public:
-    ByteData() : data_(nullptr), index_(0) {}
+    ByteData()
+        : data_(nullptr), index_(0), stored_quarters_(0), stored_byte_(0) {}
 
     // Reading from the ByteData is only allowed when a ReadingScope is on the
     // stack. This ensures that we have a DisallowHeapAllocation in place
@@ -204,6 +208,7 @@ class ConsumedPreParsedScopeData {
 
     int32_t ReadUint32();
     uint8_t ReadUint8();
+    uint8_t ReadQuarter();
 
     size_t RemainingBytes() const {
       DCHECK_NOT_NULL(data_);
@@ -213,6 +218,8 @@ class ConsumedPreParsedScopeData {
     // private:
     PodArray<uint8_t>* data_;
     int index_;
+    uint8_t stored_quarters_;
+    uint8_t stored_byte_;
   };
 
   ConsumedPreParsedScopeData();
@@ -231,11 +238,6 @@ class ConsumedPreParsedScopeData {
   // subscopes') variables.
   void RestoreScopeAllocationData(DeclarationScope* scope);
 
-  // Skips the data about skippable functions, moves straight to the scope
-  // allocation data. Useful for tests which don't want to verify only the scope
-  // allocation data.
-  void SkipFunctionDataForTesting();
-
  private:
   void RestoreData(Scope* scope);
   void RestoreDataForVariable(Variable* var);

test/cctest/scope-test-helper.h

@@ -17,10 +17,6 @@ class ScopeTestHelper {
     return var->scope()->MustAllocateInContext(var);
   }
 
-  static void AllocateWithoutVariableResolution(Scope* scope) {
-    scope->AllocateVariablesRecursively();
-  }
-
   static void CompareScopes(Scope* baseline, Scope* scope,
                             bool precise_maybe_assigned) {
     CHECK_EQ(baseline->scope_type(), scope->scope_type());
@@ -109,6 +105,20 @@ class ScopeTestHelper {
       MarkInnerFunctionsAsSkipped(inner);
     }
   }
+
+  static bool HasSkippedFunctionInside(Scope* scope) {
+    if (scope->scope_type() == ScopeType::FUNCTION_SCOPE &&
+        scope->AsDeclarationScope()->is_skipped_function()) {
+      return true;
+    }
+    for (Scope* inner = scope->inner_scope(); inner != nullptr;
+         inner = inner->sibling()) {
+      if (HasSkippedFunctionInside(inner)) {
+        return true;
+      }
+    }
+    return false;
+  }
 };
 }  // namespace internal
 }  // namespace v8