Stop Using IEnumerable The Wrong Way in .NET! | Code Cop

Поделиться
HTML-код
  • Опубликовано: 27 окт 2024

Комментарии • 99

  • @zagoskintoto
    @zagoskintoto 2 месяца назад +214

    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.

    • @nickchapsas
      @nickchapsas  2 месяца назад +37

      Ah ofc! Great catch 👏👏

    • @NoName-1337
      @NoName-1337 2 месяца назад +1

      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?

    • @Chiarettoo
      @Chiarettoo 2 месяца назад

      @@NoName-1337 You're right there won't be any resizing but the manual update ensures that the new list reflects the correct length.

  • @271kochu
    @271kochu 2 месяца назад +35

    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.

    • @zagoskintoto
      @zagoskintoto 2 месяца назад +7

      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.

  • @timseguine2
    @timseguine2 2 месяца назад +14

    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.

  • @utubekade
    @utubekade 2 месяца назад +4

    I'm impressed you reproduced their result!

  • @homeropata
    @homeropata 2 месяца назад +2

    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.

  • @haxi52
    @haxi52 2 месяца назад +5

    I include a question about `IEnumerable` in every interview I conduct, and really surprised at how many people get it wrong.

  • @shikyokira3065
    @shikyokira3065 2 месяца назад +1

    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

  • @gaborpinter4523
    @gaborpinter4523 2 месяца назад +15

    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 :)

    • @sudhansusekharsatapathy8908
      @sudhansusekharsatapathy8908 2 месяца назад +1

      IEnumerable magic happen because of the yield keyword.

    • @MrWuffels
      @MrWuffels Месяц назад

      @@sudhansusekharsatapathy8908 The yield keyword is magic using the nature of the IEnumerable. But IEnumerable itself doesn't have anything to do with yield itself

  • @IllidanS4
    @IllidanS4 2 месяца назад +1

    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.

    • @MrWuffels
      @MrWuffels Месяц назад

      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

  • @dotnetMasterCSharp
    @dotnetMasterCSharp 2 месяца назад

    This is absoulutely awesome video, Thanks!

  • @ВладиславДараган-ш3ф
    @ВладиславДараган-ш3ф 2 месяца назад +1

    You have to start exposing those "experts"

  • @davidklempfner826
    @davidklempfner826 2 месяца назад +3

    @6:30 it's annoying that it mixes units. It shows microseconds and nanoseconds for the Mean and Error columns.

  • @tkdbboy
    @tkdbboy 2 месяца назад +2

    Hello Nick, I'm everybody

  • @DxCKnew
    @DxCKnew 2 месяца назад +8

    I still don't understand why calling ConvertOrder() is slower than an inline delegate that does the exact same thing.

    • @Havie
      @Havie 2 месяца назад +3

      Same. I don’t get it? Does it call a different overload?

    • @sanD-xq8nb
      @sanD-xq8nb 2 месяца назад +1

      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..."

    • @vyrp
      @vyrp 2 месяца назад +6

      I think the problem is that `new Converter(...)` instantiates a new delegate every time, while passing a lambda reuses a cached lambda.

    • @renauddanniau676
      @renauddanniau676 2 месяца назад +1

      @@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.

    • @mattymattffs
      @mattymattffs 2 месяца назад +1

      Because of the new keyword

  • @ErazerPT
    @ErazerPT 2 месяца назад +3

    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.

  • @F1nalspace
    @F1nalspace 2 месяца назад +3

    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?

    • @TheOneAndOnlySecrest
      @TheOneAndOnlySecrest 2 месяца назад +1

      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.

    • @MrWuffels
      @MrWuffels Месяц назад

      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

  • @tarsala1995
    @tarsala1995 2 месяца назад +3

    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.

    • @_tonypacheco
      @_tonypacheco 2 месяца назад

      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

    • @tarsala1995
      @tarsala1995 2 месяца назад

      @@_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.

    • @MrWuffels
      @MrWuffels Месяц назад

      @@_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

    • @MrWuffels
      @MrWuffels Месяц назад

      @@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

    • @tarsala1995
      @tarsala1995 Месяц назад

      @@MrWuffels Thx. Any idea of where the speed gain is?

  • @theMagos
    @theMagos 2 месяца назад +2

    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...

  • @joephillips6634
    @joephillips6634 2 месяца назад

    Task drives me a little crazy. I wish there was an automatic way for it to "just work"

  • @anm3037
    @anm3037 2 месяца назад +11

    Am think about a conspiracy theory: maybe Nick is the person behind the rubbish posts, then comes on RUclips to refute it for 👍🏽

  • @aymaneeljahrani2280
    @aymaneeljahrani2280 2 месяца назад

    Isn’t it cleaner to do conversion by creating an appropriate constructor ? Or a static method FromSomething(Something s) ?

  • @halivudestevez2
    @halivudestevez2 2 месяца назад

    I thought Ienumerable is a ready made and filled thing vs IQueryable which is deferred...

    • @nickchapsas
      @nickchapsas  2 месяца назад

      Nop

    • @MrWuffels
      @MrWuffels Месяц назад +1

      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

  • @MrWuffels
    @MrWuffels Месяц назад

    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

    • @MrWuffels
      @MrWuffels Месяц назад

      @Nick Chapsas The explanation, WHY Linq is Indeed faster, is because IListProvider.ToList()-implementations are optimised with memory access, which ConvertAll isn't

    • @MrWuffels
      @MrWuffels Месяц назад

      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

    • @MrWuffels
      @MrWuffels Месяц назад

      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

  • @nikolaknezevic2117
    @nikolaknezevic2117 2 месяца назад +3

    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...

    • @JamieTwells
      @JamieTwells 2 месяца назад

      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!

    • @nikolaknezevic2117
      @nikolaknezevic2117 2 месяца назад

      @@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.

  • @Whojoo
    @Whojoo 2 месяца назад

    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.

    • @alexbagnolini6225
      @alexbagnolini6225 2 месяца назад

      Try to explicitly specify that the lambda is static.
      list.ConvertAll(static x => new NewType { Something = x.Else })

    • @Whojoo
      @Whojoo 2 месяца назад

      @@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.

    • @Whojoo
      @Whojoo 2 месяца назад

      @@alexbagnolini6225 I tried that as well, same result. Think it might just be a difference of implementation of dotnet on mac vs windows.

    • @Efemyay
      @Efemyay 2 месяца назад

      @@alexbagnolini6225 had no idea you could do this. Cool!

    • @MrWuffels
      @MrWuffels Месяц назад

      @@Efemyay static on lambdas doesn't change the compiler output. It just prevents from capturing from outer scope

  • @michal.pawlowski.3
    @michal.pawlowski.3 2 месяца назад +1

    @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; }_

    • @ytaccount8374
      @ytaccount8374 2 месяца назад +2

      This is only true for >=.NET 6. Any() does use count > 0 as well.

    • @lyndongingerich8519
      @lyndongingerich8519 2 месяца назад +2

      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.

    • @ytaccount8374
      @ytaccount8374 2 месяца назад

      @@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.

    • @lyndongingerich8519
      @lyndongingerich8519 2 месяца назад

      @@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?

    • @MrWuffels
      @MrWuffels Месяц назад

      @@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

  • @shecodes94623
    @shecodes94623 Месяц назад

    Why List in opposed to IList, can anybody tell me plz?

  • @bslushynskyi
    @bslushynskyi 2 месяца назад +7

    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.

    • @jkdmyrs
      @jkdmyrs 2 месяца назад +6

      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.

    • @ollierkul
      @ollierkul 2 месяца назад +2

      @@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.

    • @jkdmyrs
      @jkdmyrs 2 месяца назад +1

      @@ollierkul his point is that he’s unexperienced so the “Select” method name is confusing for him. I didn’t miss that.

    • @ollierkul
      @ollierkul 2 месяца назад +1

      @@jkdmyrs You're missing _something_ that's for sure

    • @tropictiger2387
      @tropictiger2387 2 месяца назад +1

      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)

  • @arjunmenon2901
    @arjunmenon2901 Месяц назад

    Exists vs any

  • @user-tk2jy8xr8b
    @user-tk2jy8xr8b 2 месяца назад +1

    Nick, what do you think on the diagnostics about using indexation instead of .First() or .Last() on arrays/lists?

    • @nickchapsas
      @nickchapsas  2 месяца назад +4

      I have a video coming next week exactly on this topic

    • @lordshoe
      @lordshoe 2 месяца назад

      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).

  • @TheOneAndOnlySecrest
    @TheOneAndOnlySecrest 2 месяца назад +8

    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.

    • @lyndongingerich8519
      @lyndongingerich8519 2 месяца назад

      Thanks! That is exactly what I was wondering.

    • @deredware9442
      @deredware9442 2 месяца назад +1

      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.

    • @adambickford8720
      @adambickford8720 2 месяца назад +1

      This is incorrect and a great example of why premature optimization is bad and doing it without numbers is pointless.

    • @lyndongingerich8519
      @lyndongingerich8519 2 месяца назад

      @@adambickford8720 Whose comment is incorrect?

    • @MrWuffels
      @MrWuffels Месяц назад

      @@lyndongingerich8519 The first one is incorrect. In this case Linq indeed does know the count of the source

  • @leopoId
    @leopoId 2 месяца назад

    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.

  • @Workshopshed
    @Workshopshed 2 месяца назад

    If you are using EFCore then Select will optimise what is pulled from the database. Is that also the case with the convertall ?

    • @jorgesoprani
      @jorgesoprani 2 месяца назад +5

      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

    • @entith
      @entith 2 месяца назад

      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.

    • @georgeokello8620
      @georgeokello8620 2 месяца назад +1

      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.

    • @MrWuffels
      @MrWuffels Месяц назад

      don't mess up EFCore Linq with System.Linq

  • @sergeyz.5845
    @sergeyz.5845 2 месяца назад +1

    👮👮👮

  • @urbanelemental3308
    @urbanelemental3308 2 месяца назад

    Facepalm.

  • @MertieKalhornrn
    @MertieKalhornrn 2 месяца назад

    I'd pay to be able to watch videos like this every day. Oh, wait, it's free✨

    • @bluecup25
      @bluecup25 2 месяца назад

      BUT BEFORE YOU MOVE ON, I'D LIKE TO LET YOU KNOW THEY JUST RELEASED 2 BRAND NEW COURSES ON DOME TRAIN