Synopse Open Source

Do you remember that the previous code was the following:

procedure TInstHashMap.RegisterWeakRef(Address: Pointer; Instance: Pointer);
var
  H: Integer;
  Item: PInstItem;
begin
  Lock;
  try
    H := Hash(Instance);
    Item := FindInstItem(Instance, H);
    if Item = nil then
      Item := AddInstItem(Instance, H);
    Item.RegisterWeakRef(Address);
  finally
    Unlock;
  end;
end;

Now (in Delphi 10 Seatle), it sounds like:

procedure TInstHashMap.RegisterWeakRef(Address: Pointer; Instance: Pointer);
var
   H, Index: Integer;
   Item: PInstItem;
begin
   if not FInitialized then Initialize;
   H := Hash(Instance);
   FBuckets[H].Lock;
   try
      Item := FindInstItem(Instance, H, Index);
      if Item = nil then
         Item := AddInstItem(Instance, H, Index);
   finally
      FBuckets[H].Unlock;
   end;
   Item.RegisterWeakRef(Address);
end;

As you can see, the giant lock has been replaced by a per-bucket lock.
So the giant lock has been replaced by 197 locks.
Much better.

They are still using TMonitor, which may be an issue in some cases, in respect to native OS mutex API.
And also the fact that the lock is released, then re-acquired in Item.RegisterWeakRef() - not optimal. A single new locked method at FBuckets[H] level would be more elegant and efficient than this mess of nested calls, for sure.
Of course, we still prefer a per-class hash list, as implemented in mORMot zeroing weak references, instead of this global hash list.

But it is a good move, in such a critical path of the RTL!
Good to see that some part of the RTL is been polished in newer versions, about performance and potential bottlenecks.

Feedback is welcome in our forum, as usual.