Skip to content

Commit 2093331

Browse files
authored
Fix assert by only accessing idx (#6924)
Asserting on `_rowCount < Utils.Size(_valueBoundaries)` was catching a case where `_rowCount`'s update was reordered before `_valueBoundaries` This was unnecessary, since this method doesn't need to use `_rowCount`. Instead, make the asserts use only `idx` which will be maintained consistent with the waiter logic in this cache. �Ensure we only ever use `_rowCount` from the caching thread, so write reordering won't matter.
1 parent a60be5f commit 2093331

File tree

1 file changed

+5
-5
lines changed

1 file changed

+5
-5
lines changed

src/Microsoft.ML.Data/DataView/CacheDataView.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1320,7 +1320,7 @@ public virtual void Freeze()
13201320

13211321
private sealed class ImplVec<T> : ColumnCache<VBuffer<T>>
13221322
{
1323-
// The number of rows cached.
1323+
// The number of rows cached. Only to be accesssed by the Caching thread.
13241324
private int _rowCount;
13251325
// For a given row [r], elements at [r] and [r+1] specify the inclusive
13261326
// and exclusive range of values for the two big arrays. In the case
@@ -1384,10 +1384,10 @@ public override void CacheCurrent()
13841384

13851385
public override void Fetch(int idx, ref VBuffer<T> value)
13861386
{
1387-
Ctx.Assert(0 <= idx && idx < _rowCount);
1388-
Ctx.Assert(_rowCount < Utils.Size(_indexBoundaries));
1389-
Ctx.Assert(_rowCount < Utils.Size(_valueBoundaries));
1390-
Ctx.Assert(_uniformLength > 0 || _rowCount <= Utils.Size(_lengths));
1387+
Ctx.Assert(0 <= idx);
1388+
Ctx.Assert((idx + 1) < Utils.Size(_indexBoundaries));
1389+
Ctx.Assert((idx + 1) < Utils.Size(_valueBoundaries));
1390+
Ctx.Assert(_uniformLength > 0 || idx < Utils.Size(_lengths));
13911391

13921392
Ctx.Assert(_indexBoundaries[idx + 1] - _indexBoundaries[idx] <= int.MaxValue);
13931393
int indexCount = (int)(_indexBoundaries[idx + 1] - _indexBoundaries[idx]);

0 commit comments

Comments
 (0)