CoreFx #16274 - LazyInitializer.EnsureInitialized overload for reference types#2868
Conversation
|
@WinCPP, |
|
|
||
| return EnsureInitializedCore<T>(ref target, valueFactory); | ||
| } | ||
| // Another occurrence at line 183. |
There was a problem hiding this comment.
I'm not sure why Volatile.Read is not used in some places, e.g. line below and it is used in EnsureInitializedCore @ line 203.
There was a problem hiding this comment.
The comment right above this explains the original reason. It is not applicable anymore. The Volatile.Read performance issue has been fixed a long time ago. Could you please delete the comment and use Volatile.Read just like in CoreCLR?
There was a problem hiding this comment.
Thanks @jkotas, not being very sure, I thought I would check as there was mix of styles. I have made changes in both the places.
| // According to the abstract ECMA CLI memory model, this read should be volatile, due to possible reorderings with | ||
| // subsequent reads from fields of "target." However, as of today (7/12/12), Volatile.Read<T> is quite slow, and in practice | ||
| // this read does *not* need to be volatile, because our memory model preserves ordering of data-dependent reads. | ||
| // Another occurrence at line 86. |
There was a problem hiding this comment.
I have changed Volatile.Read to normal read to match that at line 86. Please check the comment at line 182 and 85. Will remove those comments if inappropriate. Kindly advise.
not take boolean for tracking initialization status of the reference type object. Null check on the object parameter is sufficient to whether or not it has been initialized.
WinCPP
force-pushed
the
LazyInitializerOverloads
branch
from
1a7a7ec to
ffb31b7
Compare
|
Thanks! |
3 participants
This is port of #16274
LazyInitializer.EnsureInitialized overload for reference types that does not take boolean for tracking initialization status of the reference type object. Null check on the object parameter is sufficient to whether
or not it has been initialized.
@stephentoub @karelz please review.