Repository navigation
Improve scalability in the JoinableTask dependency chain - #1018
Conversation
…azy<T>. The problem is that ListOfOftenOne was not designed for the current usage. When we add lots of items to it, it will repeat allocating arrays. A new data structure was created to retain some advantage of the original one like more efficient to hold 0 or 1 items. (And comparing to List<T>, it saves memory for small amount of items, as we do not need any thread-safe related design here. We don't allow enumerating items directly so we don't need any reentry protection. It was completely designed to fit the current need. Consider we rarely remove a JoinableTask from a collection, and most JoinAsync will wait until the task to finish, it is rarely that we need remove any added item, so the design is to keep Add close to O(1), and remove O(n).
They were copied from tests for ListOfOftenOne to ensure new data structure to work in the similar way.
It does? I didn't think |
Andrew Arnott (AArnott)
left a comment
Andrew Arnott (AArnott)
left a comment
There was a problem hiding this comment.
Looks good overall.
no it doesn't, but it predefined fields like _syncRoot to make it easier without another wrapping, which is unfortunately a waste. It has another _version fields to detect thread-safe in the middle of enumeration (or reentry issue). Plus the extra object header. |
|
not sure what leads the copyright head error. It appeared that nothing changed since the earlier build. |
Andrew Arnott (AArnott)
changed the title
681cb0d to
20075b6
Compare
|
Lifeng Lu (@lifengl) The build break was because of an incompatibility with another change in main where stylecop rules changed the expected header in the file. You'd only see the break once your branch merged with main. I've made the necessary correction in your branch and pushed a few extra assertions to one of your new tests. It should be good to go now. |
It was found during recently memory Watson bug investigation. When product code creates lots of dependent tasks of AsyncLazy, it can lead huge amounts of arrays to be allocated in the memory, although almost of them are released, they lead GC pressure or gets into LOH in this case.
Although this usage case has its own problem, it appears that the original data structure to hold child->parent relationship by using ListOfOftenOne was no longer suitable after many design changes in this area. It creates a new array when each item is added or removed was the primary reason behind this issue.
I replaced this with a new slightly different data structure, which tries to retain some advantage of the original data structure to be memory efficient when it holds a small amount of data (which is the majority of the time). However, the new data structure works more like List, so it retains a buffer larger than number of items it holds. I didn't switch to List, because the overhead of another layer of objects, and also List has extra data structure to prevent thread-safe/reentry problems. In our usage scenario, we don't need thread-safe as JTF is using it inside a sync lock, and reentry was prevented by not exposing any enumeration directly.
As our analysis and a previous change to JoinAsync, removing item from the list is a rare operation, so i leave it to O(n) (to find the item, then we fill the hole with the last item in the list). But adding item is mostly O(1), and enumeration will be done by resetting the original collection clear. It is used only when JTF is completed. I added ToArray, which is used by both unit tests and also a diagnostic code in JTF (not in common time).