Stop Using IEnumerable The Wrong Way in .NET! | Code Cop
ฝัง
- เผยแพร่เมื่อ 11 ก.ย. 2024
- The first 200 get 30% off our new Git and GitHub Actions courses on Dometrain with code GIT30: dometrain.com/...
Subscribe to my weekly newsletter: nickchapsas.com
Become a Patreon and get special perks: / nickchapsas
Hello, everybody, I'm Nick, and in this video of Code Cop I will take a look at some bad LinkedIn advice that shows how you can write some bad code and bad benchmarks in .NET and C#.
Workshops: bit.ly/nickwor...
Don't forget to comment, like and subscribe :)
Social Media:
Follow me on GitHub: github.com/Elf...
Follow me on Twitter: / nickchapsas
Connect on LinkedIn: / nick-chapsas
Keep coding merch: keepcoding.shop
#csharp #dotnet #codecop
The reason the _size is assigned is because the constructor in which _size is passed actually sets the capacity of the List. Given how List is implemented, the size actually only changes when invoking the Add,Remove,Clear methods. It's not a direct reference to the Length of the internal array. The Capacity actually returns the Length of the array normally and is used to check if the List needs to grow the array in size.
One can think of _size as "The actual amount of items in the array" (which is not its Length since List creates bigger arrays than what it needs)
If they weren't setting the _size manually, then any other method would behave different that what you would expect because they all check for _size.
Ah ofc! Great catch 👏👏
Yes, I had this thought too, but the code uses the direct access to the internal array with the index to the elements. So there is no method called and therefore no resizing of the array will happen or did I miss something?
@@NoName-1337 You're right there won't be any resizing but the manual update ensures that the new list reflects the correct length.
7:32 Isn't that the capacity being passed into the ctor? It should initialize a list of zero elements with a backing aray of length at least _size. Naturally, you have to set the _size later.
Also, I'm sure that the original poster didn't notice the different unit of measurement, and believed the code to be "only" 4x faster. I'm used to using both the period and a comma as the decimal separator, so I didn't notice at first as well.
Yes, exactly. It's the capacity and that's why _size is set manually after. Because internally it's only handled when using Add,Remove,Clear,etc. methods otherwise.
I would tend to prefer Select alone for consistency with other IEnumerable uses. I prefer to have one more or less default way of doing certain things (unless there is another concern) because it lowers the cognitive complexity of writing and reading code.
I include a question about `IEnumerable` in every interview I conduct, and really surprised at how many people get it wrong.
If I had a nickel for every C# developer who is not aware of the nature of IEnumerable, I wouldn't comment on youtube, I would just sit on the beach drinking margaritas :)
IEnumerable magic happen because of the yield keyword.
@@sudhansusekharsatapathy8908 The yield keyword is magic using the nature of the IEnumerable. But IEnumerable itself doesn't have anything to do with yield itself
If you know the actual object type of the IEnumerable many of these can be done, but many people just don't realize that IEnumerable can also be a future object which is utilized used by Linq for performance purpose
I'm impressed you reproduced their result!
I feel proud of myself realizing early the delegate construction is the culprit. However I think I have seen plans for caching more delegate expressions in future C# versions. Also the reason for the "double" _size, but that has been explained already.
I have to tell you it's not. In the generated output it's still doing the same thing but making it even slower, as the used function is stored in a different method table
Hello Nick, I'm everybody
The ConvertAll was slower in the first example because a new delegate was created for every call. When removed, the compiler can cache it, making the consecutive calls faster.
This is absoulutely awesome video, Thanks!
You have to start exposing those "experts"
I still don't understand why calling ConvertOrder() is slower than an inline delegate that does the exact same thing.
Same. I don’t get it? Does it call a different overload?
Creo que eso tiene que ver con lo que se describe en el libro Grokking Algoritmos 1ra Ed. (Páginas 43-45) o 2da Ed. (Páginas 47-49), The Call Stack:
"...when you call a function from another function, the calling function is paused in a partially completed state. All the values of the variables for that function are still stored in memory..."
I think the problem is that `new Converter(...)` instantiates a new delegate every time, while passing a lambda reuses a cached lambda.
@@vyrp Yep I do think the same because we force a "new" and the compiler is not able to see that it could be optimized. That's also why in modern C# it is always recommended to use the "static" keyword such as 'static x => new Order(...)'. It shows clearly what we aimed for.
Because of the new keyword
Eeheh, the moment i saw IEnumerable and such an order of magnitude in the speed difference i already knew where this was going. It's such a common mistake when you forget to actually materialize it. And as benchmarks go, that's why i always go look it up when i see numbers too good to be true. It's because they usually aren't all that "true", you just missed something.
When I get the benchmark results, I copy them from the console when creating an image, omit some columns, etc. A mistake happened during copying, and that's all.
You usually read the comments on post, it's interesting that you skipped them this time.
I clearly stated there that I intentionally didn't mention Select in the text, only Select().ToList() and that I only tested Select out of curiosity, not because I think they are equivalent...
Wow it's actually your post - I always wonder if the original author ever watches their own post be torn apart by Nick! Did you not know about the lazy evaluation of IEnumerable? It's like the main feature of it!
@@JamieTwells As I mentioned, I was curious about how much time Select takes by itself, just out of curiosity. If you read the text, you'll notice that it does not mention Select, only Select.ToList(). Additionally, if I believed that Select was relevant for comparison with the ConvertAll method, I would have dedicated code snippet with only Select() in the image.
@6:30 it's annoying that it mixes units. It shows microseconds and nanoseconds for the Mean and Error columns.
Am think about a conspiracy theory: maybe Nick is the person behind the rubbish posts, then comes on TH-cam to refute it for 👍🏽
Interesting, why is the lambda passing to ConvertAll() better to use instead of a simple static function? In the end, the compiler should generate the same code anyway, or i am wrong?
That would actually be really interesting to know. I also noticed quite a big difference with using method group vs lambda functions in performance.
My best guess is that lambdas are somehow treated differently because the compiler knows they are only used there exclusively.
For example the compiler creates the instance holding this lambda delegate only once and stores it in a static field somewhere.
The generated code is not the same. For the lambda there is a neu class generated containing the anonymous method as instance method. It is indeed (very marginally) slower that using the static function
Edit: static function is indeed slower. Instance methods from the same instance however are faster sometimes
For DTO purposes on lists and arrays I would use ConvertAll. This name is more understandable to me and more readable. For me Select means take some objects from list which meets a condition and then do something about them. I wouldn't guess the purpose of converting one list to another using Select method. However, as one commentator said I would also consider using Select for sake of consistency if rest of the data interpreted as IEnumerable.
Based on your description of Select, you do not actually understand the method.
It does not “take some objects from a list which meets a condition”; that’s the “Where” method.
Select “projects” or “maps” ALL items into a new item. It’s equivalent to a JavaScript “map”.
Using “Select” is not less readable that “ConvertAll”. It’s idiomatic C#; it’s how the language is expected to be written. Not using it shows your inexperience.
@@jkdmyrs I think you missed his point. Seems to me he meant that the word "Select" intuitively means something different to him than mapping items, not that he thought the method actually does something else. Which i think is fair, because I always found it a bit weird too coming from JavaScript, but I assume it's meant to be SQL-like.
That being said, I still think Select is the best choice to use, and anyone experienced with C# will be very familiar with it and have no issue understanding what it does.
@@ollierkul his point is that he’s unexperienced so the “Select” method name is confusing for him. I didn’t miss that.
@@jkdmyrs You're missing _something_ that's for sure
It really is a shame they decided to call it Select and not Map. One of the original ideas with LINQ was that it would let you use a SQL like language to query collections. LINQ is a very good set of functional utilities, but they chose to obscure what it actually was and left us with some weird names. SelectMany is Bind/FlatMap, Where is Filter and Aggregate is fold/reduce for example (Aggregate is actually a pretty good name)
I could create a comment to tell you that the List constructor sets the capacity of the list, not the size, but I bet a thousand others have already done it, so I won't...
Actually convert all is quite useful for arrays. The issue with Select is that the ToArray / ToList don't know the count upfront so it produces more garbage down the line.
However an Array.ConvertAll / List.ConvertAll DOES know the count to allocate the new array so it only needs to allocate a single array.
You need to try this with much bigger arrays to see a real difference though 10_000 elements is nothing, and also the allocation of 10k+ objects propably outweights the time it takes to allocate an array a few times.
Thanks! That is exactly what I was wondering.
ToArray and ToList for enumerables actually do know the count in cases where it is possible. The underlying code for linq has optimizations that flow through the "size" of the dataset if it is deterministic. So in this case ToList would pre-allocate the correct size.
This is incorrect and a great example of why premature optimization is bad and doing it without numbers is pointless.
@@adambickford8720 Whose comment is incorrect?
@@lyndongingerich8519 The first one is incorrect. In this case Linq indeed does know the count of the source
Task drives me a little crazy. I wish there was an automatic way for it to "just work"
Isn’t it cleaner to do conversion by creating an appropriate constructor ? Or a static method FromSomething(Something s) ?
@nickchapsas I'm unsure if it's common knowledge, but using _.Any()_ extension method to check, if a list/array contains any elements is not the best idea. _.Count > 0_ and _.Length > 0_ perform much better. Also, _Count()_ extension method is significantly less performant than _.Count_ property, which may seem obvious, but after checking out the _.Count()_ implementation, it's not: _if (source is ICollection collectionoft) { return collectionoft.Count; }_
This is only true for >=.NET 6. Any() does use count > 0 as well.
Why are Any() and Count() less performant if they just check .Count or .Length under the hood anyway? I have wondered this for some time.
@@lyndongingerich8519 it depends on what you’re using. It will try to get the count properly to be used for lists and arrays but it won’t do the same for IEnumerable as it will use the MoveEnumerator to check if there’s anything in it.
@@ytaccount8374 All right, but you can't use .Count or .Length unless the IEnumerable is an ICollection or IList anyway. In that case, why are Any() and Count() less efficient?
@@lyndongingerich8519 it has some checks underneath, but this overhead really marginal. It's efficient enough. However, especially with Count() working on every IEnumerable, you should be cautious not to cause multiple enumerations by accident
Kind of shame that you didn't explain why there is a gain in speed. I'm not really convinced by "what normal person would do" but more about the logic behind the differences in codes.
In the original post he was passing in new converter, which means for every item in the list it called the constructor again. The lambda version is static, every item in the list reuses the same object
@@_tonypacheco Are you sure it's `static`? The static feature needs to be explicitely marked like this: `Func square = static x => x * x;`. I would more thing that it''s captured and reused.
@@_tonypacheco That's wrong. The ConvertAll method gets passed the new constructer once, at it is then called for each element.
The lambda version is also never static. It's an instance method of a newly generated class
@@tarsala1995 the static keyword for a lambda (statix x => ...) doesn't change the compiler output. It just keeps the function from capturing references from outer scope
@@MrWuffels Thx. Any idea of where the speed gain is?
Nick, what do you think on the diagnostics about using indexation instead of .First() or .Last() on arrays/lists?
I have a video coming next week exactly on this topic
I always saw it as a no-no since the indexing is right there but at the same time it *is* darn convenient (if you know the collection has at least one element in it of course).
If a piece of code that does the exact same thing is 4,000 times slower even though you didn't write it, you should realize that something is wrong.
I thought Ienumerable is a ready made and filled thing vs IQueryable which is deferred...
Nop
They're both deffered. IQueryable also is an IEnumerable. However, Queryable comes from EntityFramework and usually marks an iteratable over an extern resource, while IEnumerable marks an iteratable over ANY resource
Something's off with your benchmark here. On which .net version is it done, and is it done on a optimized release build? It doesn't seem like it.
Your explanation about the Enumerable one being terrible is of course true, but your change speeding up the ConvertAll isn't really explainable.
Removing the new Converter line? Doesn't make any difference. It's not optimized out, the opposite is the case: There will always be a "new Converter" statement generated.
Taking the lambda instead of the delegate? Is doing the same thing, only a tiny bit slower as the used function is stored in a different method table.
I came to test it myself as I don't find a good explanation, WHY your fix would have sped it up in any way.
I tested with NET 8 and on a release build with dotnet benchmark. Enumerable.Select.ToList is indeed faster.
Keep in mind Linq got some performance boosts with NET 7 and 8. It may be true that it's slower in older versions.
My tests between the ConvertAll() method calls jumped a bit, but Linq was consistently faster
@Nick Chapsas The explanation, WHY Linq is Indeed faster, is because IListProvider.ToList()-implementations are optimised with memory access, which ConvertAll isn't
I have to correct one thing: When using a static method to pass as the delegate, instead of a lambda, then it is indeed slower (function is stored in the static memory). When passing an instance method, instead of the lambda, it's marginally faster then the lambda
Actually, results vary a lot depending on where the methods and the test list is stored. If it's static, if it's on the instance and so on. However, LinQ is consistently the fastest
Exists vs any
I tried the benchmarks myself as well, but I actually get different results. Although I am using a mac, which could be the culprit in this case.
I have 42 us vs 38 us (ConvertAll vs SelectToList)
Same memory allocation as you have.
But like you said, the difference between the 2 is something you can ignore in the majority of cases.
Try to explicitly specify that the lambda is static.
list.ConvertAll(static x => new NewType { Something = x.Else })
@@alexbagnolini6225 Same result I'm afraid. But could be a difference in implementation of dotnet since I'm using mac and I believe Nick is using Windows.
@@alexbagnolini6225 I tried that as well, same result. Think it might just be a difference of implementation of dotnet on mac vs windows.
@@alexbagnolini6225 had no idea you could do this. Cool!
@@Efemyay static on lambdas doesn't change the compiler output. It just prevents from capturing from outer scope
👮👮👮
If you are using EFCore then Select will optimise what is pulled from the database. Is that also the case with the convertall ?
EF Core is not actually working with collections. It's just translating the Expressions into actual SQL and executing it.
It's not really something you can compare as ConvertAll is a collection specific method that will not really be used by EF
I would imagine no, because you'd have to call ToList() first, which will cause EF to generate and execute SQL and load the results into memory all before the call to ConvertAll occurs.
If I remember correctly, Select only will optimize for sql when the collection has IQueryable interface otherwise if it doesn’t use that EFCore interface, then the collection will have to store all the data retrieved and store it in application memory ie db scan then it applies the filters with the Select lambdas.
don't mess up EFCore Linq with System.Linq
i'm sure you're right but you talk too fast, show brief flashes of code and blast through things for me to keep up
I'd pay to be able to watch videos like this every day. Oh, wait, it's free✨
BUT BEFORE YOU MOVE ON, I'D LIKE TO LET YOU KNOW THEY JUST RELEASED 2 BRAND NEW COURSES ON DOME TRAIN
Facepalm.