kjeske / rudder-example

MIT License
0 stars 0 forks source link

Your example extension Replace method may be rather slow and allocate huge amounts of memory. #1

Open Rhywden opened 2 years ago

Rhywden commented 2 years ago

Basically, I wondered what the best method was to replace one item in an immutable list. I first used the included Replace<t>(T oldItem, T newItem) "native" method (got the oldItem through Find() or FirstOrDefault)), then also tried the overload with the IComparer and then tried your extension method which has to call .toImmutableList() at the end as Select() only returns an IEnumerable.

Created a basic ImmutableList with 10,000 entries (each entry being public record TestRecord(string Id, string Content)) and ran it through a Benchmark when trying to replace the 10,000th entry (i.e. the last one).

This is the result:

image

As you can see, it's not only considerably slower, it also has a bit of an allocation problem.

kjeske commented 2 years ago

Good benchmark. Definitely my Replace method can be improved. I will check it more detailed later, but even the following implementation should be faster:

public static IEnumerable<T> Replace<T>(this IEnumerable<T> enumerable, Func<T, bool> predicate, Func<T, T> newItem) =>
    enumerable.Select(item => predicate(item) ? newItem(item) : item);

because it enumerates enumerable only once.

Rhywden commented 2 years ago

That won't help much, I'm afraid. It's the final .toImmutableList() which yields the huge allocation and the larger execution time. Without that conversion, your replace method is actually faster (but only yields an IEnumerable) and has comparable allocation. That's how I stumbled across this in the first place ;) Without the call to .toImmutableList() the benchmark looks like this:

image