Skip to content

Commit

Permalink
Remove ConcurrencyMode.RecordIsolation (#418)
Browse files Browse the repository at this point in the history
* Remove ConcurrencyControlMode.RecordIsolation

* minor tweaks

* Preserve lock offsets in following bit positions; add AggressiveInlining to ITsavoriteSession lock methods

* Add comments on ReadCache consistency

* Update Locking.mdand Reviv.md to remove RecordIsolation

* Clean up DoTransientLocking to simply be IsLocking
  • Loading branch information
TedHartMS authored May 28, 2024
1 parent 1671184 commit ade2991
Show file tree
Hide file tree
Showing 30 changed files with 230 additions and 1,627 deletions.
5 changes: 2 additions & 3 deletions libs/storage/Tsavorite/cs/benchmark/Options.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ class Options
[Option('z', "locking", Required = false, Default = ConcurrencyControlMode.None,
HelpText = "Locking Implementation:" +
$"\n {nameof(ConcurrencyControlMode.None)} = No Locking (default)" +
$"\n {nameof(ConcurrencyControlMode.LockTable)} = Locking using main HashTable buckets" +
$"\n {nameof(ConcurrencyControlMode.RecordIsolation)} = RecordInfo locking only within concurrent IFunctions callbacks")]
$"\n {nameof(ConcurrencyControlMode.LockTable)} = Locking using main HashTable buckets")]
public ConcurrencyControlMode ConcurrencyControlMode { get; set; }

[Option('i', "iterations", Required = false, Default = 1,
Expand Down Expand Up @@ -118,7 +117,7 @@ class Options
public string GetOptionsString()
{
static string boolStr(bool value) => value ? "y" : "n";
return $"-b: {Benchmark}; d: {DistributionName.ToLower()}; n: {NumaStyle}; rumd: {string.Join(',', RumdPercents)}; reviv: {RevivificationLevel}; revivbinrecs: {RevivBinRecordCount};"
return $"b: {Benchmark}; d: {DistributionName.ToLower()}; n: {NumaStyle}; rumd: {string.Join(',', RumdPercents)}; reviv: {RevivificationLevel}; revivbinrecs: {RevivBinRecordCount};"
+ $" revivfrac {RevivifiableFraction}; t: {ThreadCount}; z: {ConcurrencyControlMode}; i: {IterationCount}; hp: {HashPacking};"
+ $" sd: {boolStr(UseSmallData)}; sm: {boolStr(UseSmallMemoryLog)}; sy: {boolStr(UseSyntheticData)}; safectx: {boolStr(UseSafeContext)};"
+ $" chkptms: {PeriodicCheckpointMilliseconds}; chkpttype: {(PeriodicCheckpointMilliseconds > 0 ? PeriodicCheckpointType.ToString() : "None")};"
Expand Down
120 changes: 9 additions & 111 deletions libs/storage/Tsavorite/cs/src/core/ClientSession/ClientSession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1178,12 +1178,7 @@ public bool SingleReader(ref Key key, ref Input input, ref Value value, ref Outp

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public bool ConcurrentReader(ref Key key, ref Input input, ref Value value, ref Output dst, ref ReadInfo readInfo, ref RecordInfo recordInfo)
{
// RecordIsolation locking is done at Transient scope, in stackCtx. For reads either an SLock or XLock is valid.
Debug.Assert(!_clientSession.store.DoRecordIsolation || readInfo.RecordInfo.IsLocked, "Expected Lock in ConcurrentReader");

return _clientSession.functions.ConcurrentReader(ref key, ref input, ref value, ref dst, ref readInfo, ref recordInfo);
}
=> _clientSession.functions.ConcurrentReader(ref key, ref input, ref value, ref dst, ref readInfo, ref recordInfo);

public void ReadCompletionCallback(ref Key key, ref Input input, ref Output output, Context ctx, Status status, RecordMetadata recordMetadata)
=> _clientSession.functions.ReadCompletionCallback(ref key, ref input, ref output, ctx, status, recordMetadata);
Expand All @@ -1197,40 +1192,14 @@ public bool SingleWriter(ref Key key, ref Input input, ref Value src, ref Value

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void PostSingleWriter(ref Key key, ref Input input, ref Value src, ref Value dst, ref Output output, ref UpsertInfo upsertInfo, WriteReason reason, ref RecordInfo recordInfo)
{
if (_clientSession.store.DoRecordIsolation)
PostSingleWriterRecordIsolation(ref key, ref input, ref src, ref dst, ref output, ref upsertInfo, reason, ref recordInfo);
else
PostSingleWriterNoLock(ref key, ref input, ref src, ref dst, ref output, ref upsertInfo, reason, ref recordInfo);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void PostSingleWriterNoLock(ref Key key, ref Input input, ref Value src, ref Value dst, ref Output output, ref UpsertInfo upsertInfo, WriteReason reason, ref RecordInfo recordInfo)
{
recordInfo.SetDirtyAndModified();
_clientSession.functions.PostSingleWriter(ref key, ref input, ref src, ref dst, ref output, ref upsertInfo, reason);
}

public void PostSingleWriterRecordIsolation(ref Key key, ref Input input, ref Value src, ref Value dst, ref Output output, ref UpsertInfo upsertInfo, WriteReason reason, ref RecordInfo recordInfo)
{
// Note: RecordIsolation XLock was taken by CASRecordIntoChain() (if not readcache), separate from the Transient-scope stackCtx.recSrc lock.
try
{
PostSingleWriterNoLock(ref key, ref input, ref src, ref dst, ref output, ref upsertInfo, reason, ref recordInfo);
}
finally
{
if (reason != WriteReason.CopyToReadCache) // readcache records are readonly so are not XLocked
recordInfo.UnlockExclusive();
}
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public bool ConcurrentWriter(long physicalAddress, ref Key key, ref Input input, ref Value src, ref Value dst, ref Output output, ref UpsertInfo upsertInfo, ref RecordInfo recordInfo)
{
// RecordIsolation locking is done at Transient scope, in stackCtx.
Debug.Assert(!_clientSession.store.DoRecordIsolation || upsertInfo.RecordInfo.IsLockedExclusive, "Expected XLock in ConcurrentWriter");

(upsertInfo.UsedValueLength, upsertInfo.FullValueLength, _) = _clientSession.store.GetRecordLengths(physicalAddress, ref dst, ref recordInfo);
if (!_clientSession.functions.ConcurrentWriter(ref key, ref input, ref src, ref dst, ref output, ref upsertInfo, ref recordInfo))
return false;
Expand All @@ -1252,33 +1221,10 @@ public bool InitialUpdater(ref Key key, ref Input input, ref Value value, ref Ou

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void PostInitialUpdater(ref Key key, ref Input input, ref Value value, ref Output output, ref RMWInfo rmwInfo, ref RecordInfo recordInfo)
{
if (_clientSession.store.DoRecordIsolation)
PostInitialUpdaterRecordIsolation(ref key, ref input, ref value, ref output, ref rmwInfo, ref recordInfo);
else
PostInitialUpdaterNoLock(ref key, ref input, ref value, ref output, ref rmwInfo, ref recordInfo);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void PostInitialUpdaterNoLock(ref Key key, ref Input input, ref Value value, ref Output output, ref RMWInfo rmwInfo, ref RecordInfo recordInfo)
{
recordInfo.SetDirtyAndModified();
_clientSession.functions.PostInitialUpdater(ref key, ref input, ref value, ref output, ref rmwInfo);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void PostInitialUpdaterRecordIsolation(ref Key key, ref Input input, ref Value value, ref Output output, ref RMWInfo rmwInfo, ref RecordInfo recordInfo)
{
// Note: RecordIsolation XLock was taken by CASRecordIntoChain(), separate from the Transient-scope stackCtx.recSrc lock.
try
{
PostInitialUpdaterNoLock(ref key, ref input, ref value, ref output, ref rmwInfo, ref recordInfo);
}
finally
{
recordInfo.UnlockExclusive();
}
}
#endregion InitialUpdater

#region CopyUpdater
Expand All @@ -1292,42 +1238,16 @@ public bool CopyUpdater(ref Key key, ref Input input, ref Value oldValue, ref Va

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void PostCopyUpdater(ref Key key, ref Input input, ref Value oldValue, ref Value newValue, ref Output output, ref RMWInfo rmwInfo, ref RecordInfo recordInfo)
{
if (_clientSession.store.DoRecordIsolation)
PostCopyUpdaterRecordIsolation(ref key, ref input, ref oldValue, ref newValue, ref output, ref rmwInfo, ref recordInfo);
else
PostCopyUpdaterNoLock(ref key, ref input, ref oldValue, ref newValue, ref output, ref rmwInfo, ref recordInfo);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void PostCopyUpdaterNoLock(ref Key key, ref Input input, ref Value oldValue, ref Value newValue, ref Output output, ref RMWInfo rmwInfo, ref RecordInfo recordInfo)
{
recordInfo.SetDirtyAndModified();
_clientSession.functions.PostCopyUpdater(ref key, ref input, ref oldValue, ref newValue, ref output, ref rmwInfo);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void PostCopyUpdaterRecordIsolation(ref Key key, ref Input input, ref Value oldValue, ref Value newValue, ref Output output, ref RMWInfo rmwInfo, ref RecordInfo recordInfo)
{
// Note: RecordIsolation XLock was taken by CASRecordIntoChain(), separate from the Transient-scope stackCtx.recSrc lock.
try
{
PostCopyUpdaterNoLock(ref key, ref input, ref oldValue, ref newValue, ref output, ref rmwInfo, ref recordInfo);
}
finally
{
recordInfo.UnlockExclusive();
}
}
#endregion CopyUpdater

#region InPlaceUpdater
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public bool InPlaceUpdater(long physicalAddress, ref Key key, ref Input input, ref Value value, ref Output output, ref RMWInfo rmwInfo, out OperationStatus status, ref RecordInfo recordInfo)
{
// RecordIsolation locking is done at Transient scope, in stackCtx.
Debug.Assert(!_clientSession.store.DoRecordIsolation || rmwInfo.RecordInfo.IsLockedExclusive, "Expected XLock in InPlaceUpdater");

(rmwInfo.UsedValueLength, rmwInfo.FullValueLength, _) = _clientSession.store.GetRecordLengths(physicalAddress, ref value, ref recordInfo);
if (!_clientSession.InPlaceUpdater(ref key, ref input, ref value, ref output, ref recordInfo, ref rmwInfo, out status))
return false;
Expand All @@ -1349,40 +1269,14 @@ public bool SingleDeleter(ref Key key, ref Value value, ref DeleteInfo deleteInf

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void PostSingleDeleter(ref Key key, ref DeleteInfo deleteInfo, ref RecordInfo recordInfo)
{
if (_clientSession.store.DoRecordIsolation)
PostSingleDeleterRecordIsolation(ref key, ref deleteInfo, ref recordInfo);
else
PostSingleDeleterNoLock(ref key, ref deleteInfo, ref recordInfo);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void PostSingleDeleterNoLock(ref Key key, ref DeleteInfo deleteInfo, ref RecordInfo recordInfo)
{
recordInfo.SetDirtyAndModified();
_clientSession.functions.PostSingleDeleter(ref key, ref deleteInfo);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void PostSingleDeleterRecordIsolation(ref Key key, ref DeleteInfo deleteInfo, ref RecordInfo recordInfo)
{
// Note: RecordIsolation XLock was taken by CASRecordIntoChain(), separate from the Transient-scope stackCtx.recSrc lock.
try
{
PostSingleDeleterNoLock(ref key, ref deleteInfo, ref recordInfo);
}
finally
{
recordInfo.UnlockExclusive();
}
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public bool ConcurrentDeleter(long physicalAddress, ref Key key, ref Value value, ref DeleteInfo deleteInfo, ref RecordInfo recordInfo, out int allocatedSize)
{
// RecordIsolation locking is done at Transient scope, in stackCtx.
Debug.Assert(!_clientSession.store.DoRecordIsolation || deleteInfo.RecordInfo.IsLockedExclusive, "Expected XLock in ConcurrentDeleter");

(deleteInfo.UsedValueLength, deleteInfo.FullValueLength, allocatedSize) = _clientSession.store.GetRecordLengths(physicalAddress, ref value, ref recordInfo);
if (!_clientSession.functions.ConcurrentDeleter(ref key, ref value, ref deleteInfo, ref recordInfo))
return false;
Expand Down Expand Up @@ -1416,37 +1310,41 @@ public void CheckpointCompletionCallback(int sessionID, string sessionName, Comm
#endregion IFunctions - Checkpointing

#region Transient locking
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public bool TryLockTransientExclusive(ref Key key, ref OperationStackContext<Key, Value> stackCtx)
{
if (!Store.DoTransientLocking)
if (!Store.IsLocking)
return true;
if (!Store.LockTable.TryLockTransientExclusive(ref key, ref stackCtx.hei))
return false;
stackCtx.recSrc.SetHasTransientXLock();
return true;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public bool TryLockTransientShared(ref Key key, ref OperationStackContext<Key, Value> stackCtx)
{
if (!Store.DoTransientLocking)
if (!Store.IsLocking)
return true;
if (!Store.LockTable.TryLockTransientShared(ref key, ref stackCtx.hei))
return false;
stackCtx.recSrc.SetHasTransientSLock();
return true;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void UnlockTransientExclusive(ref Key key, ref OperationStackContext<Key, Value> stackCtx)
{
if (!Store.DoTransientLocking)
if (!Store.IsLocking)
return;
Store.LockTable.UnlockExclusive(ref key, ref stackCtx.hei);
stackCtx.recSrc.ClearHasTransientXLock();
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void UnlockTransientShared(ref Key key, ref OperationStackContext<Key, Value> stackCtx)
{
if (!Store.DoTransientLocking)
if (!Store.IsLocking)
return;
Store.LockTable.UnlockShared(ref key, ref stackCtx.hei);
stackCtx.recSrc.ClearHasTransientSLock();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -976,10 +976,7 @@ public bool SingleReader(ref Key key, ref Input input, ref Value value, ref Outp

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public bool ConcurrentReader(ref Key key, ref Input input, ref Value value, ref Output dst, ref ReadInfo readInfo, ref RecordInfo recordInfo)
{
// Note: RecordIsolation is not used with Lockable contexts
return _clientSession.functions.ConcurrentReader(ref key, ref input, ref value, ref dst, ref readInfo, ref recordInfo);
}
=> _clientSession.functions.ConcurrentReader(ref key, ref input, ref value, ref dst, ref readInfo, ref recordInfo);

public void ReadCompletionCallback(ref Key key, ref Input input, ref Output output, Context ctx, Status status, RecordMetadata recordMetadata)
=> _clientSession.functions.ReadCompletionCallback(ref key, ref input, ref output, ctx, status, recordMetadata);
Expand All @@ -1001,7 +998,6 @@ public void PostSingleWriter(ref Key key, ref Input input, ref Value src, ref Va
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public bool ConcurrentWriter(long physicalAddress, ref Key key, ref Input input, ref Value src, ref Value dst, ref Output output, ref UpsertInfo upsertInfo, ref RecordInfo recordInfo)
{
// Note: RecordIsolation is not used with Lockable contexts
(upsertInfo.UsedValueLength, upsertInfo.FullValueLength, _) = _clientSession.store.GetRecordLengths(physicalAddress, ref dst, ref recordInfo);
if (!_clientSession.functions.ConcurrentWriter(ref key, ref input, ref src, ref dst, ref output, ref upsertInfo, ref recordInfo))
return false;
Expand Down Expand Up @@ -1050,7 +1046,6 @@ public void PostCopyUpdater(ref Key key, ref Input input, ref Value oldValue, re
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public bool InPlaceUpdater(long physicalAddress, ref Key key, ref Input input, ref Value value, ref Output output, ref RMWInfo rmwInfo, out OperationStatus status, ref RecordInfo recordInfo)
{
// Note: RecordIsolation is not used with Lockable contexts
(rmwInfo.UsedValueLength, rmwInfo.FullValueLength, _) = _clientSession.store.GetRecordLengths(physicalAddress, ref value, ref recordInfo);
if (!_clientSession.InPlaceUpdater(ref key, ref input, ref value, ref output, ref recordInfo, ref rmwInfo, out status))
return false;
Expand Down Expand Up @@ -1080,7 +1075,6 @@ public void PostSingleDeleter(ref Key key, ref DeleteInfo deleteInfo, ref Record
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public bool ConcurrentDeleter(long physicalAddress, ref Key key, ref Value value, ref DeleteInfo deleteInfo, ref RecordInfo recordInfo, out int allocatedSize)
{
// Note: RecordIsolation is not used with Lockable contexts
(deleteInfo.UsedValueLength, deleteInfo.FullValueLength, allocatedSize) = _clientSession.store.GetRecordLengths(physicalAddress, ref value, ref recordInfo);
if (!_clientSession.functions.ConcurrentDeleter(ref key, ref value, ref deleteInfo, ref recordInfo))
return false;
Expand Down Expand Up @@ -1114,6 +1108,7 @@ public void CheckpointCompletionCallback(int sessionID, string sessionName, Comm
#endregion IFunctions - Checkpointing

#region Transient locking
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public bool TryLockTransientExclusive(ref Key key, ref OperationStackContext<Key, Value> stackCtx)
{
Debug.Assert(Store.LockTable.IsLockedExclusive(ref key, ref stackCtx.hei),
Expand All @@ -1123,6 +1118,7 @@ public bool TryLockTransientExclusive(ref Key key, ref OperationStackContext<Key
return true;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public bool TryLockTransientShared(ref Key key, ref OperationStackContext<Key, Value> stackCtx)
{
Debug.Assert(Store.LockTable.IsLocked(ref key, ref stackCtx.hei),
Expand All @@ -1132,6 +1128,7 @@ public bool TryLockTransientShared(ref Key key, ref OperationStackContext<Key, V
return true;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void UnlockTransientExclusive(ref Key key, ref OperationStackContext<Key, Value> stackCtx)
{
Debug.Assert(Store.LockTable.IsLockedExclusive(ref key, ref stackCtx.hei),
Expand All @@ -1140,6 +1137,7 @@ public void UnlockTransientExclusive(ref Key key, ref OperationStackContext<Key,
+ $" Slocked {_clientSession.store.LockTable.IsLockedShared(ref key, ref stackCtx.hei)}");
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void UnlockTransientShared(ref Key key, ref OperationStackContext<Key, Value> stackCtx)
{
Debug.Assert(Store.LockTable.IsLockedShared(ref key, ref stackCtx.hei),
Expand Down
Loading

0 comments on commit ade2991

Please sign in to comment.