I am not fully understanding this logic. Can you explain what we are trying to do here?

The extension property is always the last property in PropertyCacheArray so this is checking to see if the extension property is writing or whether it is finished.

The extension property which is different than other properties because although it is a dictionary property we want to serialize each element in the dictionary as a property of the POCO instead of a dictionary element thus we need some extra state to track whether it is being serialized.

nit: Given this throws for certain large string lengths (when the return count won't fit int.MaxValue), should this be documented as an exception on the method?

If so we should also document every other Deserialize method as well. Perhaps create a doc issue?

Do the other deserialize methods have this transcoding-based exception though? If so, then yes, we should. Will create an issue on https://github.com/dotnet/dotnet-api-docs.

Why not just always use Rent? If it's too large for the pool, it'll just allocate it itself.

From the previous commit (which removed the comment):
https://github.com/dotnet/corefx/pull/41414/files/a8cd02e62d6bd2d4686ff86124fdc31e233ca42d..218e1c0d306dde4441927fdcecacd071295a145c

// and because we can avoid calling Clear().

And from above:

// For performance, avoid obtaining the actual byte count unless the memory usage may be
// higher than the threshold

avoid obtaining the actual byte count unless the memory usage may be higher than the threshold

Computing an accurate byte count doesn't prevent you from still using ArrayPool.

and because we can avoid calling Clear()

The benefit analysis of clearing all of these arrays is still not obvious to me.

Computing an accurate byte count doesn't prevent you from still using ArrayPool.

I believe the intention here is to avoid doing the extra work to get an accurate byte count when we are using the arraypool anyway and save that cost (asking for 4k from the pool vs 12k isn't that much different, so asking for the worst case is good enough). However, when allocating a regular array, over-asking by up to 3x is probably too expensive, so asking for the exact amount needed by incurring the cost of getting the exact count is worth the trade-off.

With your suggestion, we'd always have to always get the accurate byte count, regardless of whether the pool was allocating itself (because it got exhausted/we need over 1 MB), or not. Maybe that approach is good enough but there are certainly some trade-offs here. @steveharter, what are your thoughts on this?

Yes there are two things going on here:

• Avoid asking for a transcoded byte count when a threshold is surpassed
• The threshold selected happens to be the same as ArrayPool's current implementation where it just does a normal alloc, so a call to Clear() can be avoided

Given that, yes the call to Clear() is insignificant so we could just use ArrayPool always.