Add ConcurrentDictionary GetOrAdd/AddOrUpdate overloads with generic arg

[stephentoub]

Adds one overload to each of GetOrAdd and AddOrUpdate. These overloads accept a generic argument that is passed through to any invocations of the supplied delegates, enabling developers to avoid delegate/closure allocations when more input is needed than just the key or existing value. The implementations are exact copies of the existing overloads then augmented with the additional parameter and threading that through to the delegate invocations.

For AddOrUpdate, there are two existing overloads with delegates, and #394 proposes that a new overload be added for each. I went back and forth on that, and for now this PR only provide a new overload for the one that accepts two delegates, as the other just didn't seem worth it (to my recollection we also didn't discuss both of these overloads in the API review... I hadn't actually realized that a new overload was proposed for each of them). If folks feel like it's really important to have the third one, though, I can add it.

Fixes https://github.com/dotnet/corefx/issues/394.

[stephentoub]

cc: @justinvp, @AlfredoMS, @terrajobst

[justinvp]

@stephentoub, thanks for putting this together!

for now this PR only provide a new overload for the one that accepts two delegates, as the other just didn't seem worth it

I agree; I'm good with just the single new overload for AddOrUpdate.

Question: Is there a need for separate TAddArg and TUpdateArg args for AddOrUpdate? Separate args weren't in the original proposal, but I now see that the extension method example you provided had separate args. The single TArg is sufficient for the cases I've run into and I like the simplicity of the single TArg, so I'm happy with the PR as-is. Just wanted to double-check.

Minor naming question: ImmutableInterlocked.GetOrAdd has a similar overload that takes a TArg, but instead of arg it is named factoryArgument. Should the parameter name be factoryArgument here as well to be consistent? I don't have a strong preference either way.

[stephentoub]

The single TArg is sufficient for the cases I've run into and I like the simplicity of the single TArg, so I'm happy with the PR as-is.

Yeah, that was my reasoning as well (that, and if you really need different args you can package them up into a struct, and then just pay the copying cost of the one that isn't needed).

Minor naming question

No strong preference, either. @KrzysztofCwalina, suggestions?

[stephentoub]

@dotnet-bot test this please

[KrzysztofCwalina]

I agree that factoryArgument would be better. I think it clarifies what the argument is for.
Otherwise looks good! Thanks.

[stephentoub]

Thanks, guys; fixed the new parameter's name.

[stephentoub]

@dotnet-bot test this please

[justinvp]

nit: factoryArgument or factoryArg?

[stephentoub]

Oh, ok.

[stephentoub]

Let's try that again ;)

[justinvp]

Sorry I didn't catch this earlier, but should this be TryUpdateInternal to reuse the once-computed hashcode?

[stephentoub]

Yes, it should be. This was just copy-and-paste from the current overload, which highlights that it, too, is calling TryUpdate instead of TryUpdateInternal. I'll fix this one here, but submit a separate PR to master to fix the current overload.

[stephentoub]

Fixed. And opened #1833 to address the existing TryUpdate calls.