Remove new space allocation in optimizing compiler, use Symbols::FromConcat where appropriate

runtime/vm/parser.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/parser.h"
 
 #include "lib/invocation_mirror.h"
 #include "platform/utils.h"
 #include "vm/ast_transformer.h"
 #include "vm/bootstrap.h"
@@ skipping 1103 matching lines @@
   ASSERT(field.is_static());
   Thread* thread = Thread::Current();
   // TODO(koda): Should there be a StackZone here?
   Zone* zone = thread->zone();
 
   const Class& script_cls = Class::Handle(zone, field.origin());
   const Script& script = Script::Handle(zone, script_cls.script());
 
   const String& field_name = String::Handle(zone, field.name());
   String& init_name = String::Handle(zone,
-      String::Concat(Symbols::InitPrefix(), field_name));
-  init_name = Symbols::New(init_name);
+      Symbols::FromConcat(Symbols::InitPrefix(), field_name));
 
   const Function& initializer = Function::ZoneHandle(zone,
       Function::New(init_name,
                     RawFunction::kRegularFunction,
                     true,   // static
                     false,  // !const
                     false,  // !abstract
                     false,  // !external
                     false,  // !native
                     Class::Handle(field.owner()),
@@ skipping 3697 matching lines @@
   const Array& functions =
       Array::Handle(Z, Array::MakeArray(enum_members.functions()));
   cls.SetFunctions(functions);
 }
 
 
 // Add an implicit constructor to the given class.
 void Parser::AddImplicitConstructor(const Class& cls) {
   // The implicit constructor is unnamed, has no explicit parameter.
   String& ctor_name = String::ZoneHandle(Z, cls.Name());
-  ctor_name = String::Concat(ctor_name, Symbols::Dot());
-  ctor_name = Symbols::New(ctor_name);
+  ctor_name = Symbols::FromConcat(ctor_name, Symbols::Dot());
   // To indicate that this is an implicit constructor, we set the
   // token position and end token position of the function
   // to the token position of the class.
   Function& ctor = Function::Handle(Z,
       Function::New(ctor_name,
                     RawFunction::kConstructor,
                     /* is_static = */ false,
                     /* is_const = */ false,
                     /* is_abstract = */ false,
                     /* is_external = */ false,
@@ skipping 6815 matching lines @@
                                   is_setter_name
                                       ? InvocationMirror::kSetter
                                       : InvocationMirror::kMethod,
                                   NULL);  // No existing function.
   }
 
   // Closurization of instance getter, setter, method or operator.
   if (is_setter_name) {
     extractor_name = String::Concat(Symbols::SetterPrefix(), extractor_name);
   }
-  extractor_name = String::Concat(Symbols::HashMark(), extractor_name);
-  extractor_name = Symbols::New(extractor_name);
+  extractor_name = Symbols::FromConcat(Symbols::HashMark(), extractor_name);
   return new(Z) InstanceGetterNode(property_pos, primary, extractor_name);
 }
 
 
 AstNode* Parser::ParsePostfixExpr() {
   TRACE_PARSER("ParsePostfixExpr");
   String* expr_ident =
       Token::IsIdentifier(CurrentToken()) ? CurrentLiteral() : NULL;
   const intptr_t expr_pos = TokenPos();
   AstNode* expr = ParsePrimary();
@@ skipping 1345 matching lines @@
 }
 
 
 static String& BuildConstructorName(const String& type_class_name,
                                     const String* named_constructor) {
   // By convention, the static function implementing a named constructor 'C'
   // for class 'A' is labeled 'A.C', and the static function implementing the
   // unnamed constructor for class 'A' is labeled 'A.'.
   // This convention prevents users from explicitly calling constructors.
   String& constructor_name =
-      String::Handle(String::Concat(type_class_name, Symbols::Dot()));
+      String::Handle(Symbols::FromConcat(type_class_name, Symbols::Dot()));

[hausner, 2015/08/20 21:45:46]

Isn't it premature to create a symbol here? If it's a named constructor, we
create and canonicalize a string that isn't going to be used.

[srdjan, 2015/08/20 22:11:18]

Discussed offline, agreed to keep it as it is, otherwise new strings would be
allocated repeatedly. We also need this in order to prevent allocation in new
space (optimizing compiler in helper thread)

   if (named_constructor != NULL) {
-    constructor_name = String::Concat(constructor_name, *named_constructor);
+    constructor_name =
+        Symbols::FromConcat(constructor_name, *named_constructor);
   }
   return constructor_name;
 }
 
 
 // Parse a primary expression of the form new T# or new T#m.
 // Current token position is after the keyword new. Extracts the
 // anonymous or named constructor and type arguments.
 // Note that type type T has already been parsed before
 // (by ParseNewOperator()) and is guaranteed to be well-formed,
@@ skipping 596 matching lines @@
           // For example, we don't know yet whether the unresolved name
           // refers to a getter or a setter. However, it is more awkward
           // to distinuish four NoSuchMethodError cases all over the place
           // in the parser. The four cases are: prefixed vs non-prefixed
           // name, static vs dynamic context in which the unresolved name
           // is used. We cheat a little here by looking at the next token
           // to determine whether we have an unresolved method call or
           // field access.
           String& qualified_name = String::ZoneHandle(Z, prefix.name());
           qualified_name = String::Concat(qualified_name, Symbols::Dot());
-          qualified_name = String::Concat(qualified_name, ident);
-          qualified_name = Symbols::New(qualified_name);
+          qualified_name = Symbols::FromConcat(qualified_name, ident);
           InvocationMirror::Type call_type =
               CurrentToken() == Token::kLPAREN ?
                   InvocationMirror::kMethod : InvocationMirror::kGetter;
           primary = ThrowNoSuchMethodError(qual_ident_pos,
                                            current_class(),
                                            qualified_name,
                                            NULL,  // No arguments.
                                            InvocationMirror::kTopLevel,
                                            call_type,
                                            NULL);  // No existing function.
         }
       } else if (FLAG_load_deferred_eagerly && prefix.is_deferred_load()) {
         // primary != NULL.
         String& qualified_name = String::ZoneHandle(Z, prefix.name());
         qualified_name = String::Concat(qualified_name, Symbols::Dot());
-        qualified_name = String::Concat(qualified_name, ident);
-        qualified_name = Symbols::New(qualified_name);
+        qualified_name = Symbols::FromConcat(qualified_name, ident);
         InvocationMirror::Type call_type =
             CurrentToken() == Token::kLPAREN ?
                 InvocationMirror::kMethod : InvocationMirror::kGetter;
         // Note: Adding a statement to current block is a hack, parsing an
         // espression should have no side-effect.
         current_block_->statements->Add(ThrowNoSuchMethodError(
                                             qual_ident_pos,
                                             current_class(),
                                             qualified_name,
                                             NULL,  // No arguments.
@@ skipping 496 matching lines @@
 void Parser::SkipQualIdent() {
   ASSERT(IsIdentifier());
   ConsumeToken();
   if (CurrentToken() == Token::kPERIOD) {
     ConsumeToken();  // Consume the kPERIOD token.
     ExpectIdentifier("identifier expected after '.'");
   }
 }
 
 }  // namespace dart