Override TList<T>.GetIsEmpty with a more efficient and threadsafe implementation
Currently TList<T>.GetIsEmpty falls back to:
function TEnumerableBase.GetIsEmpty: Boolean;
var
enumerator: IEnumerator;
begin
enumerator := GetEnumerator;
Result := not enumerator.MoveNext;
end;
This can raise "Collection was modified; enumeration operation may not execute" exception in multi-threaded scenarios and seems not very efficient. Using the implementation Exit(fCount = 0); would solve both.
Comments (8)
-
repo owner -
repo owner - changed milestone to 1.2
- changed version to 1.2 (develop)
- changed component to Base.Collections
- edited description
-
repo owner - changed status to resolved
TList<T> directly checks fCount field in GetIsEmpty (fixes issue
#205)→ <<cset 848412abe753>>
-
reporter The entire function call is not an atomic operation.
No, and that was not what I was proposing. But it is thread-safe in a manner that it cannot fail and cannot do any harm in a multithreaded scenario, which is not the case for the old implementation. As an example, I sometimes use IsEmpty to check if it makes sense to enter the lock at all. Of course I cannot rely on the result after entering the lock. But this pattern can reduce the overall time that threads spend in critical sections, which improves scalability.
-
reporter Thanks for fixing this issue so quickly!
-
repo owner TList<T> directly checks fCount field in GetIsEmpty (fixes issue
#205)→ <<cset 2729a4eda52a>>
-
repo owner TList<T> directly checks fCount field in GetIsEmpty (fixes issue
#205)→ <<cset 7f1557a9b950>>
-
repo owner - changed version to 1.2
- Log in to comment
While I agree this can be (and will be) optimized nothing about it will make this any more thread-safe. The entire function call is not an atomic operation. And furthermore you obviously will do something else depending on the result which can already be not valid anymore.