Optimize the Read permission recalculation when a Space changes
Summary: This is currently wildly inefficient, and while I try to avoid premature optimization it seems likely to become a disaster for the Arisia Volunteers Space: with hundreds of people frequently modifying a Space with tens of thousands of records, it's likely to bog down.
This has a deadline -- before people are using the Space hard -- but in practice it's not a major issue until about November.
For now, let's focus on the low-hanging fruit. I believe that a fairly conservative set of changes can fix 98% of the problem pretty quickly. Here are the key observations:
- The relevant code is in
OldUserSpaceSession.scala
. In particular, when we get a CurrentState()
message, we call setRawState()
, and that clears the enhancedState.
- The
CurrentState()
message is insufficient in and of itself -- we will need to propagate the actual change Event that has happened, as well.
- I believe that the creation of any Thing cannot affect the readability of any other Thing. This is crucial, because a large fraction of operations are Creates. (Creating a Model or a Role can't affect Reads, because nothing else can depend on it yet.)
- I believe that the modification of an instance cannot affect the readability of any other Thing, unless that instance is a Role. The vast majority of operations are instance modifications, so this is a gigantic win.
- For now, modifications to Models, Roles, Properties, etc, should still cause a full rebuild.
Basically, only changes to Models or Roles seem likely to require a full readability rebuild. Those are extremely rare once the system is really up and running, so we should be able to make things much better.
That still leaves a lot of weight, though, because we are still checking the SystemHidden Properties on every Thing. This probably implies that the better solution would be, in most cases, to evolve the current UserSpaceSession State based on the Event, rather than replacing the full State. I believe this is likely to be wildly more efficient and more correct in the long run. (Since, in the long run, we're going to want to be propagating these Events to the Client as well.)
Note that fixing that will require a change to SpaceCore.updateState()
, as well as the CurrentState()
message and everything in between, to take an Option[List[SpaceEvent] ]
. This is what the above changes will hang off of. There are a number of cases which will not result in a SpaceEvent
, which is why it needs to be optional.
All of this should receive heavy unit testing. I suspect that the testing will require more effort than the changes themselves. But this is delicate and critical code, so hammer it hard.