ObjectPool: Create Pool Size of 100 instead of throwing on 0#163
ObjectPool: Create Pool Size of 100 instead of throwing on 0#163derrickstolee merged 2 commits intomicrosoft:masterfrom
Conversation
|
Does the LazyUTF8Pool or the BytePool need similar minimums? There multipliers are greater than 1.0 so they wouldn't cause an error but might be good just to use a minimum on them as well? Also thinking about this more and the expand might never increase if the pool is too small. |
|
@kewillford What do you think about changing my approach here and replacing the exception in |
|
@derrickstolee Yeah that is much simpler. Looks good. |
| { | ||
| LazyUTF8String.ResetPool(new MockTracer(), 0); | ||
| LazyUTF8String.FreePool(); | ||
| LazyUTF8String.InitializePools(new MockTracer(), 0); |
There was a problem hiding this comment.
Can we also validate that LazyUTF8String.Length is 100 after ResetPool and InitializePools?
There was a problem hiding this comment.
I thought about validating, but the number 100 is not special (and in the new implementation is hidden).
There was a problem hiding this comment.
You could validate that it is greater than 0
There was a problem hiding this comment.
I do think we can add a verification that the value is strictly larger than 0. Which is the important thing, and gives a clear indicator of "what are we testing here?" (instead of just "don't throw").
There was a problem hiding this comment.
(I'm not used to GitHub not updating automatically while the browser is open.)
| { | ||
| SortedFolderEntries.FreePool(); | ||
| SortedFolderEntries.InitializePools(new MockTracer(), indexEntryCount: 0); | ||
| SortedFolderEntries.ResetPool(new MockTracer(), indexEntryCount: 0); |
There was a problem hiding this comment.
Same comment here, validate that the size is 100?
A user reported an issue when cloning a repo with no files (other than
.gitignoreand.gitattributes). This is recorded by #162.The issue is that the
ObjectPoolthrows an exception when given a pool size of 0, and we multiply the number of index entries by a double less than 1.0 and cast down to anint. This is easily fixed by adding a minimum pool size. This is verified with a simple unit test.