Jump to content

Performance Patterns for Safe Event Handling


Lupine00

1,167 views

It's often the case that you need to handle a "sensitive" event in some mod code. These events are things like OnHit, OnAnimationEvent, and OnItemAdded. What these events have in common is that in some cases they can be produced in large busts.

 

An event handler that takes too long handling those events can result in an overloaded script system that overflows its stack and gets suspended, with unpleasant results for your game.

 

Another kind of event that exhibits this problem is one generated by a cloak, and it follows that trigger box events can also cause similar overloads.

 

In all cases, the problem is the same, efficient handling of the events. While some will say "don't use cloaks ever", that's a gross over-statement that takes a dogmatic approach to cloaks. Problems have arisen because the pattern shown in the CK cloak tutorial is quite bad, and adding scripts with cloaks is generally a bad idea, but cloaks in general are no worse than any other event that can fire rapidly in bursts - probably less bad than an OnItemAdded running on the player.

 

The central problem with script in these handlers is the basic problem of taking too long to execute. This is caused simply by doing too much in the handler script. Scripts that go off and potentially do heavy work are almost certain to cause an issue unless additional measures are taken.

 

A favorite pattern is the "busy state", which uses Papyrus built-in states to avoid overload. In this pattern, after a successful call to the event handler, additional calls to the handler will do nothing unless the first call has completed.

 

The code looks something like this:

Spoiler

Event OnItemAdded(Form addItemBase, Int itemCount, ObjectReference addItemReference, ObjectReference theSourceContainer)

    GoToState("Busy") ; Enter busy state - any further calls will get the other event handler

    ... do some serious work ... could take a while ...

    GoToState("") ; Leave the busy state

EndEvent

 

State Busy

    Event OnItemAdded(Form addItemBase, Int itemCount, ObjectReference addItemReference, ObjectReference theSourceContainer)

        ; Empty function does nothing.

    EndEvent

EndState

 

This approach is fine is you just want to know the event is being hit by something. It's good for situations like an OnHit event where you want to know the player is being hit in combat, but you don't need to handle every hit, just as many as you can - and the player won't really notice those hits you didn't process.

 

But, quite frequently, the reason you're running an event handler is because you need to get all the events. You really don't want to miss any. This is most likely with an OnItemAdd, where you're looking for a particular item.

 

An appropriate filter is the recommended solution to OnItemAdd performance risks, and that's great if you know exactly what items you are interested in, and the list is small. If the list is huge, or you don't know the items but simply that they have a certain keyword, or came from a certain mod, then filtering is probably unsuitable.

 

Nevertheless, where you can use a filter, that should be your first approach, as filters are very good at reducing the number of event calls, so you don't have to run a lot of code on irrelevant items at all.

 

But let's suppose a filter is not going to cut it. Imagine you are doing what I wanted to do recently, which is to detect every DD key the player obtains.

There are now many mods that create their own uniquely flavored DD keys, and so it's not enough just to check for the default DD keys any longer - if it ever was.

 

I could do this...

Spoiler

Keyword Property ddRestraintsKey Auto
Keyword Property ddChastityKey Auto
Keyword Property ddPiercingKey Auto

GlobalVariable Property _DFKeyFound Auto

 

Event OnItemAdded(Form addItemBase, Int itemCount, ObjectReference addItemReference, ObjectReference theSourceContainer)

    If _DFKeyFound.GetValue() > 0.0
        Return
    EndIf
    If addItemReference
        If addItemReference.HasKeyword(ddRestraintsKey) || addItemReference.HasKeyword(ddChastityKey) || addItemReference.HasKeyword(ddPiercingKey)
            _DFKeyFound.SetValue(1.0)
        EndIf
    Else
        If addItemBase.HasKeyword(ddRestraintsKey) || addItemBase.HasKeyword(ddChastityKey) || addItemBase.HasKeyword(ddPiercingKey)
            _DFKeyFound.SetValue(1.0)
        EndIf
    EndIf

EndEvent

There are no states needed in this case. It's not ... terrible ... the work done in the handler is strictly bounded, and once the _DFKeyFound global gets set, further calls early-out and do nothing. Though, of course, a massive inventory flood coming from emptying Sanguine's chest in SD+, or the prison confiscation chest, or similar, probably wouldn't ever set that value, and the script would run a lot of times. Possibly a couple of hundred times in a single burst - but it's not such a huge script it would bring down your game - it would just take a while to clear. But ... if you then piled more load on top, it might become an issue.

 

So, the above code would probably be ok, but it's still a little worrying.

 

 

It could be better. We could do less. What is the minimum we could do?

One approach could be this:

 

Spoiler

Event OnItemAdded(Form addItemBase, Int itemCount, ObjectReference addItemReference, ObjectReference theSourceContainer)

    StorageUtil.FormListAdd(Player, "DF_KeyRuleInventoryAddListItems", addItemReference)
    StorageUtil.FormListAdd(Player, "DF_KeyRuleInventoryAddListBases", addItemBase)
EndEvent

FormListAdd is for all intents instantaneous compared to any Papyrus execution; the cost is entirely dominated by the Papyrus overhead.

 

Somewhere else there is a regular service routine that pulls everything out of the lists and deals with it. How that's done properly is a topic of its own, and I don't want to get stuck in that, so let's just pretend it's easy to iterate over each list and process the items. One list is mostly going to be full of "None".

 

This is the least we could reasonably do in a handler, and it won't miss any items.

The down side of this is that we absolutely must service those lists reliably or they will grow without bound, and probably cause a CTD.

That is also probably an unacceptable risk. What if the Papyrus servicing the lists fails?

 

Here's the compromise I came up with for DF:

Spoiler

Event OnItemAdded(Form addItemBase, Int itemCount, ObjectReference addItemReference, ObjectReference theSourceContainer)

    ; The obvious risk here is that we don't service this list and it grows without bound...
    ; So don't allow that.
    If StorageUtil.FormListCount(Player, "DF_KeyRuleInventoryAddList") < 1000
        If addItemReference
            StorageUtil.FormListAdd(Player, "DF_KeyRuleInventoryAddList", addItemReference)
        Else
            StorageUtil.FormListAdd(Player, "DF_KeyRuleInventoryAddList", addItemBase)
        EndIf
    EndIf

EndEvent

In this case, if the list is overflowing, we back off and stop adding - items will be lost - but this is better than creating a massively bloated list that is going to lag in processing anyway.

In fact, with a limit of 1000, if we are skipping filling the list, it's almost certainly because the servicing routine has stopped running and is dead.

Also, I took a little Papyrus time to determine whether to handle as item or base. This is a less clear-cut whether it's the right approach, but it saves time later in processing the list later, that is for sure.

 

The alternative is to use the approach above and fill two lists (or the same list) with both inputs and discard all the None items later. Another approach is to discard non-uniques, which will filter out all the None items at the point of add - but those calls to StorageUtil are going to start using a bit more CPU if you do that. What's best? You'd need to profile in detail to know. I couldn't be bothered going that far this time.

 

So, there is no straight answer to any problem, and no perfect solution. But sometimes the "busy" state pattern is not going to do what you want, and you need something else.

Busy state invariably discards a lot of calls when you get a burst. It maintains performance, but the cost is lots of lost events.

 

States can result in some hierarchic resolution of the state declarations, so they are more costly than they look, but I suspect it's still negligible. The problem is really losing all those calls that you made the handler to catch in the first place.

 

Without detailed profile results it's hard to say exactly what would be the absolute best approach here; but profiling this sort of scenario in a meaningful way is not trivial. I can easily imagine some garbage profile results, but a useful measure might be how often we get a suspended stack.

6 Comments


Recommended Comments

It depends what you're trying to achieve. In many cases, a cloak is fine - they've just been demonized by people who have received wisdom about cloaks and repeat it without considering that cloaks are very dependent on the specific implementation.

 

If you treat cloak code the way I suggest you treat burstable events, chances are you'll never have a problem with your cloak.

But there are mods that do make bad use of cloaks, and mods that made seriously bad use of cloaks and created the bad reputation they have.

 

SLAR replaced SLA's cloak with a quest that populates aliases based on conditions.

The quest approach can only fill as many aliases as the quest has, so the work is always bounded.

 

A cloak could potentially generate a list of 50 NPCs, but SLA* only wants to process a small number of NPCs, so finding those extra NPCs is a bad outcome for SLA.

However, SLA* should theoretically be processing all those NPCs, it's just designed not to, to help performance.

If you can process the NPCs efficiently, maybe you do want to find them.

 

A lot of mods - and DF is currently one - find rapists by using a cloak and then picking the nearest NPCs. (Lozeak copied the cloak example from the CK site, and that example is the root cause of most bad cloaks you'll see; despite its warnings it demonstrates a bad pattern). I plan to remove the DF cloak once I have a solution I like in SLAX.

 

Simplistically, in the case DF had, it would have been better to use a quest, because you could directly fill the sex partner aliases.

 

SLD uses a cloak to do it, where the cloak script is very fast, but it's something I might revisit. In this case SLD wants to find an arbitrarily large number of participants. SLD will happily set up a massive chain-run on the PC, and will also re-scan for participants during an ongoing scene. Solving this with a quest probably wouldn't be practical or efficient.

 

How I intend to solve this problem for SLAX is something I've been thinking about a long time. The existing quest based approach has some problems. It takes a long time to start a quest, and it's actually quite a load-generating process. Probably, the load from spinning up a quest is more than from a cloak that finds the same number of people and uses the busy-state model to turn itself off once it's found them. Certainly, the latency for quest start is high.

 

I want a more responsive approach, and more continuity in the alias population. I want the aliases to update incrementally as the player moves, or NPCs move away, or closer. That can't be done with a quest.

 

For a single cell, it's possible to write C++ that walks the occupants and applies conditions purely in C++, which is extremely fast and responsive. So that would work for interiors, but outside, the player may be at a cell boundary. In that case, finding the adjacent cells can be hard. I looked at how DynDOLOD does it, and it has a big table that tells it what cells are adjacent to others, which is not a something I want to replicate due to the work involved. This means that a hybrid approach might be best.

 

It's an area where I need to do some experimentation.

 

But the short answer is that in many cases a cloak can be replaced with a quest, or a better made cloak, or if you just want one NPC, there's a built-in native function to find a random NPC within a specified radius.

 

If your goal really is to add a new script to every NPC (or worse, every ObjRef) in a radius, you might want to rethink that entire approach, because doing that is what is most likely to generate suspended stacks in Papyrus when using a cloak. But if you bounded the scripts that could be added using the busy pattern, it might be perfectly fine.

Link to comment

One approach you can use is the game.Find* functions. FindRandomActorFromRef will grab an actor inside a certain radius of the given reference. This is useful if you don't have too many condition on what sort of actor you want and you only want a small number. For example, the rapist case above could pick a random actor and throw away unsuitable actors and still find a candidate fairly  fast. The advantage here is twofold. Firstly, you're doing this without the event burst of a cloak or the stat up time of a quest, and secondly, you don't have to surrender control while the quest starts and you can keep your code all in one function. Downside is that if you have too many conditions on what sort of actor you need, you may spend too long testing and discarding actors. If that happens a quest is better.

 

Formlists are also potentially useful. They act as Sets, so if you don't need to worry about duplicates. So for the case where a cloak adds actors to a list for processing, a Formlist saves worrying about duplicates.

 

You can also use the FindRandom* functions with a formlist in some cases. Suppose you want a cloak-like function, but that it's not essential that you get all of them, or that they all happen right away.  You could have a timer loop that every second adds one or two actors into a formlist, Doesn't matter which ones because you'll get them all eventually.

Link to comment
8 hours ago, DocClox said:

One approach you can use is the game.Find* functions. FindRandomActorFromRef will grab an actor inside a certain radius of the given reference. This is useful if you don't have too many condition on what sort of actor you want and you only want a small number. For example, the rapist case above could pick a random actor and throw away unsuitable actors and still find a candidate fairly  fast. The advantage here is twofold. Firstly, you're doing this without the event burst of a cloak or the stat up time of a quest, and secondly, you don't have to surrender control while the quest starts and you can keep your code all in one function. Downside is that if you have too many conditions on what sort of actor you need, you may spend too long testing and discarding actors. If that happens a quest is better.

I did mention that approach if you want to identify low numbers of, or single NPCs.

 

I plan to experiment with it for the SLAX scanner, trying an incremental update approach, at least for outdoors. Indoors C++ cell scanning may be better, but outdoors you run into the cell boundary issue. I think I wrote about this somewhere else though so I'm repeating myself. 

 

The advantage of find - I think - is that you don't put any burst pressure on Papyrus, and it can just chug along single-threaded in its Papyrussy way, and Papyrus basically can't ever stress or overload your game with a single thread of execution. The find native functions may come at a cost though, and it may be more efficient to get more actors at once if scanning with C++. If I know how to get adjacent cells in C++ that would be a huge help.

 

Ah, it's like finding all the info about effects on the player. It's been done in C++, but there's no source, so you need to put in a lot of effort to figure out how it is done, and it might require decoding data that SKSE knows nothing about.

Link to comment
22 minutes ago, Lupine00 said:

I did mention that approach if you want to identify low numbers of, or single NPCs.

 

You did. My bad.

 

22 minutes ago, Lupine00 said:

Ah, it's like finding all the info about effects on the player. It's been done in C++, but there's no source, so you need to put in a lot of effort to figure out how it is done, and it might require decoding data that SKSE knows nothing about.

 

I'll be interested in your results. I have some custom detects that need to call a pap function to evaluate whether the actor shows up in the detect, (or what color they highlight) and that uses a lot of these techniques. The end result is performant enough , but it can take a couple of casts before new actors get fully updated. I'd be interested in anything that lets me improve on that.

Link to comment

I think the performance edge of cloaks may be under-rated; if used with the busy pattern, probably quite safe.

 

Given how you have to use them over a time-period, my suspicion is that cloaks are relying on data that build up spatial data as part of the render-prep phase, and that then allows highly efficient query of what actors are nearby, have LOS, etc.  So you're waiting for a while, then suddenly the data is there, because the render is in a separate thread. So, the cloak updates what is in range at that point, with latency.

 

But that's just a hunch based on the way that cloak results trickle in. They may not work anything like that. They may simply be iterating through all the actors in the cell, and then nearby cells if needed.

 

Either way, from a Papyrus viewpoint, the cloak is perfect for finding "all the things" in a radius around a target.

The problems come when it finds too many things, and also because it keeps on finding them, periodically - which I suspect is another feature that Beth tried to implement quite carefully, so cloak-based damage shields can work predictably.

 

This latter behavior may be what creates all the latency in cloaks, so they may not be connected to render at all. But as noted, this was Beth's best shot at using the engine to search for collections for actors in a radius. If only they'd exposed a way to do this once-only on demand.

Link to comment
×
×
  • Create New...