Not clear to me whether this has been resolved on the list or not.

Added Area-Library, Triaged labels.

We have found a solution, but it might be interesting for the VM devs to investigate if there are any low hanging fruits for the 50x factor.
CCing Florian and reassigning to VM.

The mixin implementation just sets the length to length+1 and then assigns into the new slot. My expectation would be that this is slower than the native addAll but not 50x.

cc @fsc8000.
Removed Area-Library label.
Added Area-VM label.

cc @sgmitrovic.

Removed Priority-Unassigned label.
Added Priority-Low label.

VMs growable array length setter does not grow efficiently:
    if (new_length > _capacity) {
      _grow(new_length);
    }
...

In contrast, 'add' method:
    if (len == _capacity) {
      _grow(len * 2);
    }

The performance issue would be fixed if we change ListMixin.addAll to something like:
  void addAll(Iterable<E> iterable) {
    int old = this.length;
    this.length = old + iterable.length;
    for (E element in iterable) {
      this[old++] = element;
    }
  }

We have decided that VM library's length setter should not grow the list aggressively as we would loose the ability to control the memory growth.

cc @floitschG.
Removed Area-VM label.
Added Area-Library label.

We generally prefer not to run through the iterator twice. We could call iterable.toList() and then use the list instead, but that would cost in memory.
Of course, we could also test (is List) if the iterable is a List or Set where it is safe to call length.
We could do a length *= 2 ourselves to get better performance but that seems bad too.

I.e., this is another case where we could do better if we knew that .length was efficient, but we don't, so we can't. Testing for List is a hack which misses a lot of other good length-known collections.

To clarify: .length is quite efficient. It is the growing and the included copying of the data that is expensive. In the suggested solution you would still be much faster with this[this.length++] if you primed the capacity of 'this' by setting it to the maxed size once:

  void addAll(Iterable<E> iterable) {
    int old = this.length;
    this.length = old + iterable.length;
    this.length = old;
    for (E element in iterable) {
      this[this.length++] = element;
    }
  }

I think this is an example where sharing code is bad as it includes significant performance penalty: some iterables have expensive length increase past the capacity, some iterables have expensive length computation. That is why the user must override certain ListMixin methods (as VM-s library do) in order to get acceptable performance. Maybe this bug should be marked as 'Works as designed'?

I was referring to the .length of the Iterable argument to addAll.
The problem is that Iterable.length can be either efficient (List, Set, Queue, a lot of other ones) or expensive (e.g., list.filter((x)=>true) where we don't know the length without running through it and calling the test on each element), but we don't have an easy way to tell.
That is why we end up with code like this one, where addAll adds elements one at a time because it doesn't know how many elements to add in total before its done.

When I talk about an "efficient length", we have considered (but never really accepted) adding a "fastLength" getter to Iterable that will always return in constant time, but may return null if it can't find a result that fast. That way we would be able to know in advance how many elements the iterable has, if the iterable knows it itself, and still leave room for lazy iterables that require a full traversal to know the length. It's not pretty, though.

One solution for the current problem could be to increment length by more than one (e.g., double it before starting, and then setting it back to the final count when we are done).
Another option is to have "addAll" call "add" repeatedly, and document it, and then assume that repeated add operations are efficient (because that is something we optimizie for). As stated elsewhere, I try to avoid having one overridable method call another if possible, but this case may have to be an exception.

Let's document this case in the dartdocs of the mixin and close this bug.
I think we have explored all possible options and the initial goal (verifying that nobody is doing anything stupid that could be easily fixed) has been achieved.

Added AsDesigned label.

Created issue #12840 to track the documentation update process.