How I can Optimize this linq query

Question

Is there some way to optimize this query ?

_alternatives.Cast<AlternativePartName>()
                                .Where(alt => original.ToLower() == alt.CompName)
                                .Select(alt => alt.AltCompName).ToList();

I am profiling my application and this code is one of the bottlenecks 196 ms but it executed a lot of times.

don't optimize the query, but cache the result? That said, did you plug the provider to a log text writer, in order to see which queries are actually executed? — Steve B, Jul 02 '14 at 13:01
Is it possible to not use `.ToList()`? Instead enumerate over the items in the subsequent code. — Herman, Jul 02 '14 at 13:05
Is this some kind of Key/Value stores? Are you using database or is this just objects? — Euphoric, Jul 02 '14 at 13:05
also, why do you have to cast the items and do you implement a custom cast operator? — Lorentz Vedeler, Jul 02 '14 at 13:10

score 5 · Accepted Answer · answered Jul 02 '14 at 13:02

5

Instead of calling ToLower for each item, call it only once:

var lower = original.ToLower();

_alternatives.Cast<AlternativePartName>()
              .Where(alt =>  lower == alt.CompName)
              .Select(alt => alt.AltCompName).ToList();

answered Jul 02 '14 at 13:02

Selman Genç

100,147
13
119
184

Also can use `IEnumerable` instead of `ToList()` – Vivek Parekh Jul 02 '14 at 13:06
IEnumerable is an interface. If you change the type to be IEnumerable there'll still be a List object which will be created and assigned to the IEnumerable. IEnumerable in itself can't hold any object. – Sidharth Panwar Jul 02 '14 at 13:15
@Selman22: Even if I call ToLower() on the collection it'll still work on each object in the collection, isn't it? – Sidharth Panwar Jul 02 '14 at 13:21
@sloth: I got your point. What you're saying is that there may be conditions where we may not iterate the result and in these cases ToList won't be required. Agreed, I thought that would be a trivial case but point taken. – Sidharth Panwar Jul 02 '14 at 13:24
@SidharthPanwar yes, it will compare each item with the lowerest version of `original`, instead of converting original to lower-case every time. – Selman Genç Jul 02 '14 at 13:25
@Selman22: I'd rather go for String.Equals unless I need my collection values to be in lower case. – Sidharth Panwar Jul 02 '14 at 13:27
@SidharthPanwar Almost. If you iterate the result once, there may be no point in creating a `List<...>` and thus copying the data around. But if you iterate the result more than once, it will probably be better to store the result in a `List<...>` than executing the query multiple times. – sloth Jul 02 '14 at 13:30
@sloth: You're almost right. I got the crux of what you're saying. Here's a good link - http://stackoverflow.com/questions/4180767/whats-the-difference-between-liststring-and-ienumerablestring. Check the reply marked as answer. – Sidharth Panwar Jul 02 '14 at 13:38
@SidharthPanwar *You're almost right. I got the crux of what you're saying.* I don't think so. I reckon you disregard the deferred execution of Linq queries. – sloth Jul 02 '14 at 13:50
@sloth: Got both your points - Deferred execution and the time we save with using IEnumerable over List. – Sidharth Panwar Jul 02 '14 at 13:52

score 2 · Answer 2 · edited Jul 02 '14 at 13:13

2

You should try to use more of your cores with .AsParallel() - this could be a improvement at huge lists with long strings

string lower = original.ToLower();
_alternatives.Cast<AlternativePartName>()
              .AsParallel()
              .Where(alt =>  lower == alt.CompName)
              .Select(alt => alt.AltCompName)

edited Jul 02 '14 at 13:13

WhileTrueSleep

1,524
1
19
32

answered Jul 02 '14 at 13:09

fubo

44,811
17
103
137

1

This is a great option, IMO. But be sure that your data set is big enough to take advantage of the parallelization. For smaller sets it'll slow down the query even more. Here's a good link - http://www.dotnetperls.com/asparallel – Sidharth Panwar Jul 02 '14 at 13:19

score 0 · Answer 3 · answered Jul 02 '14 at 13:06

0

I'm assuming that "original" and "CompName" are strings. If yes, then you should do the following:

_alternatives.Cast<AlternativePartName>()
                                .Where(alt => String.Equals(original, alt.CompName, StringComparison.OrdinalIgnoreCase))
                                .Select(alt => alt.AltCompName).ToList();

answered Jul 02 '14 at 13:06

Sidharth Panwar

4,606
6
27
36

why you think it will be faster ? – Night Walker Jul 02 '14 at 13:07
You'll be saving the cost of running ToLower() on each object in your collection. The bigger your collection is the more time you'll be spending on running ToLower(). Also, the standard way of comparing strings is using String.Equals and not the == operator. – Sidharth Panwar Jul 02 '14 at 13:09

score 0 · Answer 4 · answered Jul 02 '14 at 13:08

Using correct data structures is best way to increse performance. In your case, it would be better if you used dictionary with CompName being the key and list of AltCompNames being the item. The lookup would be blazing fast, because there would be no iteration, just single lookup. And keeping such simple structure up-to-date with adding and removing items should not be a huge problem.

score 0 · Answer 5 · answered Jul 02 '14 at 13:10

0

To call orignal.ToLower() only once won't change anything real in performance.

Try to use e.g. Parallel LINQ to get the result faster if you got enough CPU-Power left.

answered Jul 02 '14 at 13:10

WhileTrueSleep

1,524
1
19
32

score -2 · Answer 6 · answered Jul 02 '14 at 13:09

-2

I am not so good with the dot syntax, but this

.Select(alt => alt.AltCompName)

combined with this

_alternatives.Cast<AlternativePartName>()

Could be achived with the select directly making the AlternativPartName

select new AlternativePartName(){...}

answered Jul 02 '14 at 13:09

Thomas Koelle

3,416
2
23
44

How I can Optimize this linq query

6 Answers6