Commit 1769c703 authored by vogelheim's avatar vogelheim Committed by Commit bot

[parser/ast] Reduce memory consumption for AstConsString.

The past re-factoring inadvertently increased memory consumption for
AstConsString. This implements a micro-optimization to revert and slightly
improve beyond the original state.

Example, Zone size for parsing closure.js:
  - 20,999,848 B (before refactoring)
  - 21,651,056 B (after refactoring patch; 3.1% regression)
  - 20,641,320 B (after this CL; 1.7% improvement over original)

(Reason: ZoneLinkedList requires 4 pointers to support
the std::list functionality (Zone*, head/tail ptr, payload ptr).
But since we only append and iterate in order and have the Zone*
available in the context, a super simple linked list (value + next ptr)
saves a bit of memory, especially for the common case of having 0 or 1
string segments.)

BUG=v8:6902, chromium:706935

Review-Url: https://codereview.chromium.org/2792353002
Cr-Commit-Position: refs/heads/master@{#44385}
parent 2db1f598
...@@ -156,12 +156,17 @@ bool AstRawString::Compare(void* a, void* b) { ...@@ -156,12 +156,17 @@ bool AstRawString::Compare(void* a, void* b) {
} }
void AstConsString::Internalize(Isolate* isolate) { void AstConsString::Internalize(Isolate* isolate) {
Handle<String> tmp(isolate->factory()->empty_string()); if (IsEmpty()) {
for (const AstRawString* current : strings_) { set_string(isolate->factory()->empty_string());
// AstRawStrings are internalized before AstConsStrings, so *current is return;
// already internalized. }
// AstRawStrings are internalized before AstConsStrings, so
// AstRawString::string() will just work.
Handle<String> tmp(segment_.string->string());
for (AstConsString::Segment* current = segment_.next; current != nullptr;
current = current->next) {
tmp = isolate->factory() tmp = isolate->factory()
->NewConsString(tmp, current->string()) ->NewConsString(current->string->string(), tmp)
.ToHandleChecked(); .ToHandleChecked();
} }
set_string(tmp); set_string(tmp);
...@@ -285,19 +290,19 @@ const AstRawString* AstValueFactory::GetString(Handle<String> literal) { ...@@ -285,19 +290,19 @@ const AstRawString* AstValueFactory::GetString(Handle<String> literal) {
} }
AstConsString* AstValueFactory::NewConsString() { AstConsString* AstValueFactory::NewConsString() {
AstConsString* new_string = new (zone_) AstConsString(zone_); AstConsString* new_string = new (zone_) AstConsString;
DCHECK_NOT_NULL(new_string); DCHECK_NOT_NULL(new_string);
AddConsString(new_string); AddConsString(new_string);
return new_string; return new_string;
} }
AstConsString* AstValueFactory::NewConsString(const AstRawString* str) { AstConsString* AstValueFactory::NewConsString(const AstRawString* str) {
return NewConsString()->AddString(str); return NewConsString()->AddString(zone_, str);
} }
AstConsString* AstValueFactory::NewConsString(const AstRawString* str1, AstConsString* AstValueFactory::NewConsString(const AstRawString* str1,
const AstRawString* str2) { const AstRawString* str2) {
return NewConsString()->AddString(str1)->AddString(str2); return NewConsString()->AddString(zone_, str1)->AddString(zone_, str2);
} }
void AstValueFactory::Internalize(Isolate* isolate) { void AstValueFactory::Internalize(Isolate* isolate) {
......
...@@ -126,12 +126,24 @@ class AstRawString final : public ZoneObject { ...@@ -126,12 +126,24 @@ class AstRawString final : public ZoneObject {
class AstConsString final : public ZoneObject { class AstConsString final : public ZoneObject {
public: public:
AstConsString* AddString(const AstRawString* s) { AstConsString* AddString(Zone* zone, const AstRawString* s) {
if (s && !s->IsEmpty()) strings_.push_back(s); if (s->IsEmpty()) return this;
if (!IsEmpty()) {
// We're putting the new string to the head of the list, meaning
// the string segments will be in reverse order.
Segment* tmp = new (zone->New(sizeof(Segment))) Segment;
*tmp = segment_;
segment_.next = tmp;
}
segment_.string = s;
return this; return this;
} }
bool IsEmpty() const { return strings_.empty(); } bool IsEmpty() const {
DCHECK_IMPLIES(segment_.string == nullptr, segment_.next == nullptr);
DCHECK_IMPLIES(segment_.string != nullptr, !segment_.string->IsEmpty());
return segment_.string == nullptr;
}
void Internalize(Isolate* isolate); void Internalize(Isolate* isolate);
...@@ -143,7 +155,7 @@ class AstConsString final : public ZoneObject { ...@@ -143,7 +155,7 @@ class AstConsString final : public ZoneObject {
private: private:
friend class AstValueFactory; friend class AstValueFactory;
explicit AstConsString(Zone* zone) : next_(nullptr), strings_(zone) {} AstConsString() : next_(nullptr), segment_({nullptr, nullptr}) {}
AstConsString* next() const { return next_; } AstConsString* next() const { return next_; }
AstConsString** next_location() { return &next_; } AstConsString** next_location() { return &next_; }
...@@ -156,7 +168,11 @@ class AstConsString final : public ZoneObject { ...@@ -156,7 +168,11 @@ class AstConsString final : public ZoneObject {
String** string_; String** string_;
}; };
ZoneLinkedList<const AstRawString*> strings_; struct Segment {
const AstRawString* string;
AstConsString::Segment* next;
};
Segment segment_;
}; };
enum class AstSymbol : uint8_t { kHomeObjectSymbol }; enum class AstSymbol : uint8_t { kHomeObjectSymbol };
......
...@@ -64,9 +64,9 @@ const AstConsString* FuncNameInferrer::MakeNameFromStack() { ...@@ -64,9 +64,9 @@ const AstConsString* FuncNameInferrer::MakeNameFromStack() {
} }
// Add name. Separate names with ".". // Add name. Separate names with ".".
if (!result->IsEmpty()) { if (!result->IsEmpty()) {
result->AddString(ast_value_factory_->dot_string()); result->AddString(zone(), ast_value_factory_->dot_string());
} }
result->AddString(names_stack_.at(pos).name); result->AddString(zone(), names_stack_.at(pos).name);
} }
return result; return result;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment