[A17] A warning to modders: xpath performance

Started by NoImageAvailable, May 26, 2017, 01:40:44 PM

Previous topic - Next topic

NoImageAvailable

A17 has been out for a while now and people are starting to adapt xpath patches into their mods. However, pretty much every mod I've seen does a major mistake: they begin their xpath expressions with //.

Do not use //-expressions for your patches!

They absolutely tank performance. Just how bad is it, you ask? On my machine I can start vanilla Rimworld and get to the main menu in ~7 seconds. After I updated a large-ish overhaul mod to A17 and xpath loading times went up to 3 minutes. People on slower machines have reported even longer startup times of up to 10 minutes. That's 10 minutes startup time from a single mod!

If you use something like //ThingDef[defName="someDefName"] the system will open a file, start at the root node, find the first ThingDef and check all its child nodes to see if one of them matches <defName>someDefName</defName>. Then it will move through those child nodes again and check to see if any of them are called ThingDef. If the child nodes have their own child nodes it will check those as well. The system will iterate through every single XML node in existance, through every file and every loaded mod to find any nodes called ThingDef.

Not only does this incur a massive performance hit, it gets worse the more defs there are. So if you have a mod that adds more defs then every single one of your //-expressions becomes even more performance intensive!

Instead:

Set every one of your xpath's directly to the root node of the file you're targeting. For most files that's going to be [Defs] but other files might have other root nodes. For example a TraderKindDef might look like this:

<?xml version="1.0" encoding="utf-8"?>
<DefPackage-TraderKindDef>

  <TraderKindDef>
    <defName>Base_Neolithic_Standard<defName>
    <stockGenerators>
      [...]
    </stockGenerators>
  </TraderKindDef>

</DefPackage-TraderKindDef>


To add a new stockGenerator your xpath should look like this:
*/TraderKindDef[defName="Base_Neolithic_Standard"]/stockGenerators

or like this:
/DefPackage-TraderKindDef/TraderKindDef[defName="Base_Neolithic_Standard"]/stockGenerators

but not like this:
//TraderKindDef[defName="Base_Neolithic_Standard"]/stockGenerators

I mentioned earlier that I was looking at startup times of 3 minutes. After converting all //-type xpaths to the proper format the time went all the way down to 18 seconds. Its literally 10 times faster than before. That said, it is still almost thrice the loading time of vanilla, so still a significant performance impact. So make sure you keep to the proper format and minimize the amount of patch operations you perform, or else bring a nice book to read because you're going to stare at that startup screen a long time once you get a decent amount of mods together.

A note to tutorial makers: please update your tutorials to use proper xpath format. People are emulating the examples you give them and if you're teaching them to use //-expressions for everything then we will soon see a flood of poorly optimized mods kneecapping performance.

Update: as has been suggested by Zhentar, you can also use xpaths like */ThingDef to achieve the same result. Technically speaking using the root node proper is still marginally faster but the difference is too small to really notice in practice and this way you don't need to look up the exact node name for every file you're patching.
"The power of friendship destroyed the jellyfish."

Xnope

Thanks mate -- I hadn't noticed but yeah my xpath patch did increase my startup time by about 30 seconds -- and that was just with one patch to a vanilla def. Hopefully this reaches all patchers out there.
My W.I.P. mods:
Carnivale: GitHub | Forum Post
XnopeCore: GitHub

cuproPanda

I've noticed a slowdown as well. I thought it was just the new system of loading the patches and changing the defs, but this explains it.
cuproPanda's Mods: Survivalist's Additions, Additional Joy Objects, Cupro's Drinks, Quarry, Cupro's Stones, Zen Garden, Cupro's Alloys, Preset Filtered Zones, & more!

Shinzy


NoImageAvailable

I've updated the OP with some new information: you can also use */ in place of the root node to achieve the same result without having to look up root nodes for a bunch of different files.
"The power of friendship destroyed the jellyfish."

minimurgle

Thanks for putting this out there. I've already updated mine but I can see a few mistakes I made
Don't mind the questions. I'm probably just confused.

AdamM88

So that's why it takes like 3-4 minutes for my RimWorld to load. Balls!
Thanks for the update. Hopefully this will get remedied very soon, as I rather dislike spending 30 minutes per day staring at a loading screen. :(

Shinzy

Quote from: AdamM88 on May 26, 2017, 10:29:50 PM
So that's why it takes like 3-4 minutes for my RimWorld to load. Balls!
Thanks for the update. Hopefully this will get remedied very soon, as I rather dislike spending 30 minutes per day staring at a loading screen. :(
Whole lot of modders have already optimized their patching afaik
make sure all your mods are up to date and so

meeeko

You Sir are a life saver. I run a ton of mods and wondered why I was hitting huge fps drops and a long load time. I run a ssd raid set up and could not for the life of me figure it out. I came across your post and looked at the mods I had and this was diff the cause. I unsubbed those that used those expressions  and bam back to normal. Cheers mate.

Spdskatr

So... Just putting this out there... If I were to use C# to replace the def after it was all loaded, would this way be the fastest? Compared to PatchOperations, I'm doing a targeted System.Collections.Generic.Dictionary search and then directly editing the fields with the same accuracy as a PatchOperation.
My mods

If 666 is evil, does that make 25.8069758011 the root of all evil?

alkor

Quote from: Spdskatr on May 27, 2017, 09:12:12 PM
So... Just putting this out there... If I were to use C# to replace the def after it was all loaded, would this way be the fastest? Compared to PatchOperations, I'm doing a targeted System.Collections.Generic.Dictionary search and then directly editing the fields with the same accuracy as a PatchOperation.
Being somewhat of a necromancer, and you probably got your answer long ago already, for posteriority's sake:

Yes, it would be the fastest approach. You are effectively operating in-memory on native objects, as opposed to in-memory representations of XML documents (which are somewhat more complex than one level deep lists or dictionaries). There is a caveat you have to remember though: patch operations are effectively preprocessing, whereas the objects you work with in C# (unless you hook into the core resolver) are ones that have already been resolved - inherited, overridden, so forth. During a patch, many defs with the same defNames can co-exist in the XML structure (each in a different mod package). After resolution there'll be at most one given def type with a given defName, so naturally its lookup will be faster.

But.

This effectively invalidates load order resolution on the XML level, because when you do that, everyone who wants to tweak your values further (be it in a public mod, or for their own personal use locally), is effectively forced by you to not only load in their modifications after your mod, you are also forcing them to write their own C# assembly and apply their changes there. So not only do they have to at the very least understand C# basics, they need to figure out your code, and then they also need to understand the lifecycle of game objects, hook calling order, and so on. Well, you get the picture :)

Also, this is (theoretically) a double-edged sword. If you wanted to remove a def from the database, it would be faster do to this in a patch than after resolution, because it prevents the removed node(s) from being resolved at all (they don't exist anymore once they arrive at the resolver for processing). If you were to do it programatically in C#, then you would be adding overhead by removing something that went through the whole patching-resolution process. Theoretically.

Practically, it depends on how heavy the mod load is, how many patches are getting applied to the values you're interested in (...). You'd have to benchmark on a case by case basis, which is not feasible. Which in turn means this pretty much falls into the micro-optimizations tantrum, and instead of worrying about performance, you should pick the most apparent, easiest to read solution. I'd argue patches are easy to read, especially for others, and even coding not-so-savvy players. As opposed to digging through, let alone decompiling, your code to find out what you are changing.

Nightinggale

#11
Another xPath performance pitfall

You should not have multiple Operation for patching the same xml file. Use sequences instead. However one sequence can't patch more than one xml file.

Wrong: (itemA and itemB are in the same xml file)
<Patch>
<Operation Class="PatchOperationAdd">
<xpath>/Defs/ThingDef[defName = "itemA"]/recipes</xpath>
<value>
whatever
</value>
</Operation>
<Operation Class="PatchOperationAdd">
<xpath>/Defs/ThingDef[defName = "itemB"]/recipes</xpath>
<value>
whatever
</value>
</Operation>
</Patch>


Correct: (itemA and itemB are in the same xml file)
<Patch>
<Operation Class="PatchOperationSequence">
<success>Always</success>
<operations>
<li Class="PatchOperationAdd">
<xpath>/Defs/ThingDef[defName = "itemA"]/recipes</xpath>
<value>
whatever
</value>
</li>
<li Class="PatchOperationAdd">
<xpath>/Defs/ThingDef[defName = "itemB"]/recipes</xpath>
<value>
whatever
</value>
</li>
</operations>
</Operation>
</Patch>


The reason is that each patch file is applied to every single xml file inside Defs. When a patch file is applied, it tries to apply each Operation. The first example has two Operations, which mean xPath is checked twice for each Def xml file.

The second has just one Operation. It starts by searching for itemA and only if it is found will it search for itemB. This mean all the files not being patched can be checked in half the time.

The number of def xml files is fairly high. It's almost 400 for core alone and when adding mods, it quickly exceed 1000. This mean it doesn't really matter how much time you spend while patching a file. Instead the performance is mainly determined by how much time you spend on figuring out the patch is working with the wrong file. 20 PatchOperations in sequence will be around twice as fast as two PatchOperations, which aren't in sequence.

However the nature of PatchOperationSequence mean you have to ensure that the items you patch are in fact in the same file. If itemA and itemB aren't in the same file, itemB will not be patched because the search for itemA will fail.

If you release a mod where you expect other mods to patch it, do consider putting the "likely to patch" data in as few files as possible, preferably one if it isn't too ugly. This will allow patching using fewer sequences.

While this is about reducing the number of needed xPath searches, the point in the first post still applies as this is regarding the time spend for each xPath search. Those two "fixes" work well together and you have to use both to the full extend to get the fastest possible patch file.
ModCheck - boost your patch loading times and include patchmods in your main mod.

Nightinggale

When writing xpath search patterns, always start with Def/

Benchmarks of time usage for the different options:





Def/baseline
/Def/+1.4%
*/Def/+6.4%
//+749%
ModCheck - boost your patch loading times and include patchmods in your main mod.

kaptain_kavern

Example case:

For a very little mod of mine, with only a single simple patch, I made some measurement as well - Thanks to indications from a certain Nightinggale  8)


And it turns out that switching from

<Operation Class="PatchOperationAdd">
<xpath>Defs/ThingDef[defName = "Sandbags"]</xpath>
<value>
<minifiedDef>MinifiedFurniture</minifiedDef>
</value>
</Operation>


to

<Operation Class="PatchOperationSequence">
<success>Always</success>
<operations>
<li Class="PatchOperationTest">
<xpath>Defs/ThingDef[defName = "Sandbags"]/minifiedDef</xpath>
<success>Invert</success>
</li>
<li Class="PatchOperationAdd">
<xpath>Defs/ThingDef[defName = "Sandbags"]</xpath>
<value>
<minifiedDef>MinifiedFurniture</minifiedDef>
</value>
</li>
</operations>
</Operation>


make the game loading 1713ms quicker on my modest computer, with just this mod activated.


I was very surprised by this number ... I was ready to take the quicker method even for a couple of ms ^^