summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--tools/gn/function_template.cc3
-rw-r--r--tools/gn/scope.cc12
-rw-r--r--tools/gn/scope.h5
-rw-r--r--tools/gn/template.cc6
-rw-r--r--tools/gn/template.h20
-rw-r--r--tools/gn/template_unittest.cc18
6 files changed, 38 insertions, 26 deletions
diff --git a/tools/gn/function_template.cc b/tools/gn/function_template.cc
index 13d0186..368dc97 100644
--- a/tools/gn/function_template.cc
+++ b/tools/gn/function_template.cc
@@ -167,8 +167,7 @@ Value RunTemplate(Scope* scope,
return Value();
}
- scope->AddTemplate(template_name,
- scoped_ptr<Template>(new Template(scope, function)));
+ scope->AddTemplate(template_name, new Template(scope, function));
return Value();
}
diff --git a/tools/gn/scope.cc b/tools/gn/scope.cc
index 437b31f..b2d8d66 100644
--- a/tools/gn/scope.cc
+++ b/tools/gn/scope.cc
@@ -45,7 +45,6 @@ Scope::Scope(const Scope* parent)
Scope::~Scope() {
STLDeleteContainerPairSecondPointers(target_defaults_.begin(),
target_defaults_.end());
- STLDeleteContainerPairSecondPointers(templates_.begin(), templates_.end());
}
const Value* Scope::GetValue(const base::StringPiece& ident,
@@ -124,17 +123,17 @@ Value* Scope::SetValue(const base::StringPiece& ident,
return &r.value;
}
-bool Scope::AddTemplate(const std::string& name, scoped_ptr<Template> templ) {
+bool Scope::AddTemplate(const std::string& name, const Template* templ) {
if (GetTemplate(name))
return false;
- templates_[name] = templ.release();
+ templates_[name] = templ;
return true;
}
const Template* Scope::GetTemplate(const std::string& name) const {
TemplateMap::const_iterator found = templates_.find(name);
if (found != templates_.end())
- return found->second;
+ return found->second.get();
if (containing())
return containing()->GetTemplate(name);
return NULL;
@@ -288,10 +287,7 @@ bool Scope::NonRecursiveMergeTo(Scope* dest,
}
// Be careful to delete any pointer we're about to clobber.
- const Template** dest_template = &dest->templates_[i->first];
- if (*dest_template)
- delete *dest_template;
- *dest_template = i->second->Clone().release();
+ dest->templates_[i->first] = i->second;
}
return true;
diff --git a/tools/gn/scope.h b/tools/gn/scope.h
index 208ee55..1b0246d 100644
--- a/tools/gn/scope.h
+++ b/tools/gn/scope.h
@@ -10,6 +10,7 @@
#include "base/basictypes.h"
#include "base/containers/hash_tables.h"
+#include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"
#include "base/memory/scoped_vector.h"
#include "tools/gn/err.h"
@@ -137,7 +138,7 @@ class Scope {
// AddTemplate will fail and return false if a rule with that name already
// exists. GetTemplate returns NULL if the rule doesn't exist, and it will
// check all containing scoped rescursively.
- bool AddTemplate(const std::string& name, scoped_ptr<Template> templ);
+ bool AddTemplate(const std::string& name, const Template* templ);
const Template* GetTemplate(const std::string& name) const;
// Marks the given identifier as (un)used in the current scope.
@@ -305,7 +306,7 @@ class Scope {
scoped_ptr<PatternList> sources_assignment_filter_;
// Owning pointers, must be deleted.
- typedef std::map<std::string, const Template*> TemplateMap;
+ typedef std::map<std::string, scoped_refptr<const Template> > TemplateMap;
TemplateMap templates_;
ItemVector* item_collector_;
diff --git a/tools/gn/template.cc b/tools/gn/template.cc
index 11dcf1f..29a6b09 100644
--- a/tools/gn/template.cc
+++ b/tools/gn/template.cc
@@ -24,12 +24,6 @@ Template::Template(scoped_ptr<Scope> scope, const FunctionCallNode* def)
Template::~Template() {
}
-scoped_ptr<Template> Template::Clone() const {
- // We can make a new closure from our closure to copy it.
- return scoped_ptr<Template>(
- new Template(closure_->MakeClosure(), definition_));
-}
-
Value Template::Invoke(Scope* scope,
const FunctionCallNode* invocation,
const std::vector<Value>& args,
diff --git a/tools/gn/template.h b/tools/gn/template.h
index 0176862..0b123e0 100644
--- a/tools/gn/template.h
+++ b/tools/gn/template.h
@@ -8,6 +8,7 @@
#include <vector>
#include "base/basictypes.h"
+#include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"
class BlockNode;
@@ -17,7 +18,14 @@ class LocationRange;
class Scope;
class Value;
-class Template {
+// Represents the information associated with a template() call in GN, which
+// includes a closure and the code to run when the template is invoked.
+//
+// This class is immutable so we can reference it from multiple threads without
+// locking. Normally, this will be assocated with a .gni file and then a
+// reference will be taken by each .gn file that imports it. These files might
+// execute the template in parallel.
+class Template : public base::RefCountedThreadSafe<Template> {
public:
// Makes a new closure based on the given scope.
Template(const Scope* scope, const FunctionCallNode* def);
@@ -25,11 +33,6 @@ class Template {
// Takes ownership of a previously-constructed closure.
Template(scoped_ptr<Scope> closure, const FunctionCallNode* def);
- ~Template();
-
- // Makes a copy of this template.
- scoped_ptr<Template> Clone() const;
-
// Invoke the template. The values correspond to the state of the code
// invoking the template.
Value Invoke(Scope* scope,
@@ -42,12 +45,13 @@ class Template {
LocationRange GetDefinitionRange() const;
private:
+ friend class base::RefCountedThreadSafe<Template>;
+
Template();
+ ~Template();
scoped_ptr<Scope> closure_;
const FunctionCallNode* definition_;
-
- DISALLOW_COPY_AND_ASSIGN(Template);
};
#endif // TOOLS_GN_TEMPLATE_H_
diff --git a/tools/gn/template_unittest.cc b/tools/gn/template_unittest.cc
index 13ec848..905dc7b 100644
--- a/tools/gn/template_unittest.cc
+++ b/tools/gn/template_unittest.cc
@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include "base/strings/string_number_conversions.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "tools/gn/test_with_scope.h"
@@ -73,3 +74,20 @@ TEST(Template, UnusedVarInInvokerShouldThrowError) {
input.parsed()->Execute(setup.scope(), &err);
EXPECT_TRUE(err.has_error());
}
+
+// Previous versions of the template implementation would copy templates by
+// value when it makes a closure. Doing a sequence of them means that every new
+// one copies all previous ones, which gives a significant blow-up in memory.
+// If this test doesn't crash with out-of-memory, it passed.
+TEST(Template, MemoryBlowUp) {
+ TestWithScope setup;
+ std::string code;
+ for (int i = 0; i < 100; i++)
+ code += "template(\"test" + base::IntToString(i) + "\") {}\n";
+
+ TestParseInput input(code);
+
+ Err err;
+ input.parsed()->Execute(setup.scope(), &err);
+ ASSERT_FALSE(input.has_error());
+}
generated by cgit v1.1 at 2025-03-13 09:24:39 +0000