Delete ImmutableHashMap #31408
Delete ImmutableHashMap #31408
Conversation
|
For those following along at home, I'm not going to merge this until I can do an RPS run internally, and right now we're still sorting things out internally. |
|
pre-approved , conditional on what we find on RPS run |
|
@jasonmalinowski can this be moved forward with? |
This is because both implementations use the same inefficient storage strategy. 😄 I'm working on improving this space but this PR wouldn't block that work. |
jasonmalinowski
force-pushed
the
dev/jasonmal/delete-immutablehashmap
branch
from
53f4af1
to
8802c4b
Compare
|
|
jasonmalinowski
force-pushed
the
dev/jasonmal/delete-immutablehashmap
branch
from
8802c4b
to
c6cf7d9
Compare
|
@sharwell It's been so long I don't even remember how "slow" the standard one is in this case....IIRC there was little to no cost with the standard one but will look again. |
| @@ -227,19 +226,40 @@ public bool ContainsAnalyzerConfigDocument(DocumentId documentId) | |||
| /// Get the document in this project with the specified document Id. | |||
| /// </summary> | |||
| public Document? GetDocument(DocumentId documentId) | |||
| => ImmutableHashMapExtensions.GetOrAdd(ref _idToDocumentMap, documentId, s_tryCreateDocumentFunction, this); | |||
| { | |||
| if (ContainsDocument(documentId)) | |||
There was a problem hiding this comment.
ImmutableHashMapExtensions.GetOrAdd had an important behavior that meant if the lookup into the dictionary missed, it'd call the creation function, and if it returned null would then return null without adding the item. The standard ImmutableInterlocked.GetOrAdd does not do such a thing; this pattern is the pattern we use in Solution.GetProject so I'm using it here until we have data to argue for something better/different.
This doesn't seem to be notably faster than just using the framework library; in a case where we're doing an excessive number of GetProject calls this only is saving us a few dozen milliseconds over all of loading Roslyn.sln.
jasonmalinowski
force-pushed
the
dev/jasonmal/delete-immutablehashmap
branch
from
c6cf7d9
to
ebaf812
Compare
…-immutablehashmap
|
@jasonmalinowski can we move forward with this? |
|
Obsoleted by #72520. |
This doesn't seem to be notably faster than just using the framework library; in a case where we're doing an excessive number of GetProject calls this only is saving us a few dozen milliseconds over all of loading Roslyn.sln.