Skip to content

Conversation

@Mayo66
Copy link

@Mayo66 Mayo66 commented Nov 1, 2025

Summary

As of now, most collection operations require being called on the framework thread despite not needing access to game objects.

However, It is only when assigning via AssignTemporaryCollection(), in which it accesses (ObjectManager)objects[actorIndex] when framework thread is needed.

By making the internal data structures thread-safe, we can move collection management operations off the framework thread when safe and in doing so avoiding framework thread bottlenecks and reduce hitches, while avoiding desynchronizations between multiple consumers.

Changes

  • _collectionsByLocal: Changed to ConcurrentDictionary for lock free concurrent access
  • _collections: Added locking to protect concurrent modifications
  • CurrentCollectionId: Implemented atomic increment
  • Discovery / Change handlers: Snapshot collections before iteration to prevent concurrent modification exceptions
  • Collection Addition: Extracted collection addition logic to into dedicated Add() for clarity.

This should make the following IPC methods safe for off-framework access:

  • CreateTemporaryCollection()
  • DeleteTemporaryCollection()

AssignTemporaryCollection() still needs framework access as it is the crucial step when a collection is associated with a game object.

Misc

There was also a duplicate operation at line 189 in the original file, method RemoveCollection():
_collectionsByLocal.Remove(collection.Identity.LocalId);
at line 183 in the original file, Delete(collection); is called, which is simply

public void Delete(ModCollection collection)
        => _collectionsByLocal.Remove(collection.Identity.LocalId);

The call at line 189 looks like a duplicate, and this PR also removes said duplicate operation.

… collection logic; removed duplicate calls

Adding / removing collections do not require framework thread access.
It is only when assigning via AssignTemporaryCollection(), it needs to access `(ObjectManager)objects[actorIndex]` when framework thread is needed.
@Mayo66
Copy link
Author

Mayo66 commented Nov 1, 2025

now increments upon generating a new id to avoid scenarios when two threads call an CreateFrom method at the same time and one overwriting the other

@Mayo66 Mayo66 force-pushed the refactor/collection-storage-thread-safety branch 2 times, most recently from ebfbb42 to 12542fe Compare November 1, 2025 21:13
@Mayo66
Copy link
Author

Mayo66 commented Nov 1, 2025

added locking to public methods

@Mayo66 Mayo66 force-pushed the refactor/collection-storage-thread-safety branch from 12542fe to 0bf9fde Compare November 1, 2025 21:17
Maintainers expect _currentCollectionIdValue to start at 1, and I don't want to change that
sorry, haskell slipping through.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant