Addressed review comments from previous CLs; adding more Symbols::FromConcat

runtime/vm/flow_graph_compiler.cc

 // Copyright (c) 2013, 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/globals.h"  // Needed here to get TARGET_ARCH_XXX.
 
 #include "vm/flow_graph_compiler.h"
 
 #include "vm/bit_vector.h"
 #include "vm/cha.h"
@@ skipping 700 matching lines @@
                                        assembler()->CodeSize(),
                                        deopt_id,
                                        token_pos,
                                        CurrentTryIndex());
 }
 
 
 void FlowGraphCompiler::AddStaticCallTarget(const Function& func) {
   ASSERT(func.IsZoneHandle());
   static_calls_target_table_.Add(
-      StaticCallsStruct(assembler()->CodeSize(), &func, NULL));
+      new StaticCallsStruct(assembler()->CodeSize(), &func, NULL));

[koda, 2015/08/21 17:21:14]

Now we need to fetch the current zone from TLS and perform at least one new zone
allocation for every insertion (+ growing the array).

This will be a (slight) regression in terms of speed. What do we gain by this
change, except perhaps a couple of words of memory when there are zero entries?

[srdjan, 2015/08/21 17:33:41]

Using new(zone()), thanks for spotting it. Having one instead of three words
allows GrowableArray to grow faster.

[hausner, 2015/08/21 17:41:20]

You mean there is less data to copy when it grows? How about starting with a
larger array so you can avoid the first 2 or 3 growth cycles?

I don't feel strongly about this aspect of the change, but I question whether it
makes sense to micro-optimize pieces like this. So you have less data to copy,
but each access to an array element now requires an additional indirection, and
you have less data locality. Is this really faster?

[srdjan, 2015/08/21 18:12:13]

In this case it is probably not measurably faster. We can go either way. Choose
this way since passing objects by value always makes me itchy ;-). Some code
reviewers think this is better, some think it is not. I guess it is my call
unless there is a clear disadvantage of this approach (apart from the fact that
it changes code).

 }
 
 
 void FlowGraphCompiler::AddStubCallTarget(const Code& code) {
   ASSERT(code.IsZoneHandle());
   static_calls_target_table_.Add(
-      StaticCallsStruct(assembler()->CodeSize(), NULL, &code));
+      new StaticCallsStruct(assembler()->CodeSize(), NULL, &code));
 }
 
 
 void FlowGraphCompiler::AddDeoptIndexAtCall(intptr_t deopt_id,
                                             intptr_t token_pos) {
   ASSERT(is_optimizing());
   ASSERT(!intrinsic_mode());
   CompilerDeoptInfo* info =
       new CompilerDeoptInfo(deopt_id,
                             ICData::kDeoptAtCall,
@@ skipping 249 matching lines @@
 
 
 void FlowGraphCompiler::FinalizeStaticCallTargetsTable(const Code& code) {
   ASSERT(code.static_calls_target_table() == Array::null());
   const Array& targets = Array::Handle(zone(), Array::New(
       (static_calls_target_table_.length() * Code::kSCallTableEntryLength),
       Heap::kOld));
   Smi& smi_offset = Smi::Handle(zone());
   for (intptr_t i = 0; i < static_calls_target_table_.length(); i++) {
     const intptr_t target_ix = Code::kSCallTableEntryLength * i;
-    smi_offset = Smi::New(static_calls_target_table_[i].offset);
+    smi_offset = Smi::New(static_calls_target_table_[i]->offset);
     targets.SetAt(target_ix + Code::kSCallTableOffsetEntry, smi_offset);
-    if (static_calls_target_table_[i].function != NULL) {
+    if (static_calls_target_table_[i]->function != NULL) {
       targets.SetAt(target_ix + Code::kSCallTableFunctionEntry,
-          *static_calls_target_table_[i].function);
+          *static_calls_target_table_[i]->function);
     }
-    if (static_calls_target_table_[i].code != NULL) {
+    if (static_calls_target_table_[i]->code != NULL) {
       targets.SetAt(target_ix + Code::kSCallTableCodeEntry,
-          *static_calls_target_table_[i].code);
+          *static_calls_target_table_[i]->code);
     }
   }
   code.set_static_calls_target_table(targets);
   INC_STAT(isolate(), total_code_size, targets.Length() * sizeof(uword));
 }
 
 
 // Returns 'true' if code generation for this function is complete, i.e.,
 // no fall-through to regular code is needed.
 void FlowGraphCompiler::TryIntrinsify() {
@@ skipping 805 matching lines @@
 
 
 void FlowGraphCompiler::FrameStateClear() {
   ASSERT(!is_optimizing());
   frame_state_.TruncateTo(0);
 }
 #endif
 
 
 }  // namespace dart