Use zone allocated C-strings instead of String heap objects to construct signature symbols

runtime/vm/symbols.cc

Index: runtime/vm/symbols.cc
diff --git a/runtime/vm/symbols.cc b/runtime/vm/symbols.cc
index 3fcba6f8cf201285ed4107ea71adfe3ecebbbb0b..b4809dcb907d81f1b034a7bfbea005c503a2b310 100644
--- a/runtime/vm/symbols.cc
+++ b/runtime/vm/symbols.cc
@@ -393,6 +393,25 @@ RawString* Symbols::FromConcat(const String& str1, const String& str2) {
 }
 
 
+RawString* Symbols::FromConcatAll(const GrowableArray<const String*>& strs) {
+  intptr_t len_sum = 0;
+  for (intptr_t i = 0; i < strs.length(); i++) {
+    len_sum += OS::SNPrint(NULL, 0, "%s", strs[i]->ToCString());

[koda, 2015/08/26 17:37:06]

There's a lot of temporary allocations and duplicate work here. Some suggestions
for your consideration:

1. Overestimate the buffer length, since it's a temporary zone allocation
anyway.

2. Cache the results from ToCString to reduce duplicate work and temporary
allocations.

3. Use Utf8::Length or plain strlen instead of SNPrint.

(It would be nice if we could use String::ToUTF8 instead, but then we should
probably first move the "one-byte char fast case" from String::ToCString into
String::ToUTF8.)

[srdjan, 2015/08/26 19:43:55]

1. Overestimate the buffer length: there is always the danger (synthetic code),
that we do not allocate enough space.

2.  & 3. are both good suggestions: holding onto the cons char* generated by
ToCString and using strlen

[koda, 2015/08/26 20:02:03]

I don't mean "a large enough constant".

I mean compute 2 x (sum of one-byte string lengths) + 4 x (sum of two-byte
string lengths).

I'll leave it up to you.

[srdjan, 2015/08/26 20:23:42]

I'll leave it as it is.

+  }
+  Zone* zone = Thread::Current()->zone();
+  intptr_t buffer_len = len_sum + 1;  // Space for terminating character.
+  char* buffer = zone->Alloc<char>(buffer_len);
+  char* orig_buffer = buffer;

[koda, 2015/08/26 17:37:06]

const char* const orig_buffer

[koda, 2015/08/26 20:02:03]

Ditto?

[srdjan, 2015/08/26 20:23:42]

Done.

+  for (intptr_t i = 0; i < strs.length(); i++) {
+    intptr_t len = OS::SNPrint(buffer, buffer_len, "%s", strs[i]->ToCString());
+    buffer += len;
+    buffer_len -= len;
+    ASSERT(buffer_len >= 0);
+  }
+  return Symbols::New(orig_buffer, len_sum);

[koda, 2015/08/26 17:37:06]

What kind of hit ratio do we expect here? (If it's high, we could consider
adding ability to perform lookup from the pieces without materializing the
concatenation.)

[koda, 2015/08/26 17:37:06]

If you decide to overestimate the buffer size, you can replace 'len_sum' with
'buffer - orig_buffer' here.

[srdjan, 2015/08/26 19:43:55]

Added a TODO in case this turns out to be hot code. Original code generated
Strings instead of Symbols. From reading the code, it makes sense to me to make
the signatures symbols. 

+}
+
+
 // StringType can be StringSlice, ConcatString, or {Latin1,UTF16,UTF32}Array.
 template<typename StringType>
 RawString* Symbols::NewSymbol(const StringType& str) {