Serialization shouldn't modify the keys of a map it's iterating over

pkg/serialization/lib/src/basic_rule.dart

 // 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.
 
 part of serialization;
 
 // TODO(alanknight): Figure out how to reasonably separate out the things
 // that require reflection without making the API more awkward. Or if that is
 // in fact necessary. Maybe the tree-shaking will just remove it if unused.
 
@@ skipping 124 matching lines @@
    * the protocol compatible. See [configureForLists]/[configureForMaps].
    */
    createStateHolder() =>
      useMaps ? new _MapWrapper(fields.contents) : new List(fields.length);
 
   /**
    * Wrap the state if it's passed in as a map, and if the keys are references,
    * resolve them to the strings we expect. We leave the previous keys in there
    * as well, as they shouldn't be harmful, and it costs more to remove them.
    */
-  makeIndexableByNumber(state) {
-    if (!(state is Map)) return state;
-    // TODO(alanknight): This is quite inefficient, and we do it twice per
-    // instance. If the keys are references, we need to turn them into strings
-    // before we can look at indexing them by field position. It's also eager,
-    // but we know our keys are always primitives, so we don't have to worry
-    // about their instances not having been created yet.
-    for (var each in state.keys) {
-      if (each is Reference) {
-        var inflated = each.inflated();
-        state[inflated] = state[each];
-      }
-    }
-    return new _MapWrapper.fromMap(state, fields.contents);
-  }
+   makeIndexableByNumber(state) {
+     if (!(state is Map)) return state;
+     // TODO(alanknight): This is quite inefficient, and we do it twice per
+     // instance. If the keys are references, we need to turn them into strings
+     // before we can look at indexing them by field position. It's also eager,
+     // but we know our keys are always primitives, so we don't have to worry
+     // about their instances not having been created yet.
+     var newState = new Map();

[Jennifer Messerly, 2013/01/28 19:45:00]

one idea that might help, if the perf here is noticeable, you could scan the map
first for a Reference, and avoid allocating the new one unless you know
something needs to be replaced.

+     for (var each in state.keys) {
+       var newKey = (each is Reference) ? each.inflated() : each;
+       newState[newKey] = state[each];
+     }
+     return new _MapWrapper.fromMap(newState, fields.contents);
+   }
 
   /**
    * Extract the state from [object] using an instanceMirror and the field
    * names in [fields]. Call the function [callback] on each value.
    */
   extractState(object, Function callback) {
     var result = createStateHolder();
     var mirror = reflect(object);
 
     keysAndValues(fields).forEach(
@@ skipping 454 matching lines @@
   _MapWrapper(this.fieldList) : _map = new Map();
   _MapWrapper.fromMap(this._map, this.fieldList);
 
   operator [](key) => _map[fieldList[key].name];
 
   operator []=(key, value) { _map[fieldList[key].name] = value; }
   get length => _map.length;
 
   asMap() => _map;
 }