Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve performance when loading load order #24

Closed
1 task done
Ortham opened this issue Nov 23, 2015 · 4 comments
Closed
1 task done

Improve performance when loading load order #24

Ortham opened this issue Nov 23, 2015 · 4 comments

Comments

@Ortham
Copy link
Owner

Ortham commented Nov 23, 2015

Plugin validity checks are pretty much the only expensive operation in libloadorder, as they involve reading the whole plugin file, which can be quite large and so reading can be slow. The performance of actual plugin reading is up to libespm, but there are a couple of things libloadorder can do to improve its performance:

  • At the moment, validity is checked then the plugin is loaded, which involves two file reads (though the second only reads the file header). Given that libloadorder only deals with file headers, this could be combined into a single step, where a Plugin object is constructed, and if an exception is thrown then it is handled as an invalid plugin, and if not that constructed object is stored. Profiling shows this doesn't bring any performance benefit, and it has less clear semantics, so this idea is dropped.
  • When the load order is reloaded because filesystem changes have been detected, all data is discarded and re-loaded. Depending on the changes though, it could be that most or all of the existing plugin objects can be reused. Plugin modification times would need to be compared with their cached times to see if the objects need re-loading or not.
@Utumno
Copy link

Utumno commented Nov 23, 2015

where a Plugin object is constructed, and if an exception is thrown then it is handled as an invalid plugin, and if not that constructed object is stored

Of course that constructed object should be a few KBs big if not less

Go for a static cache in the plugin class. I have commented on this somewhere - key should be modtime, size, name - value a slim Plugin instance - IIRC

Utumno referenced this issue in Utumno/libloadorder Nov 23, 2015
This unfortunatelly proved an API issue - see #12
Note that in LIBLO_METHOD_TEXTFILE Data/ mod time was never set to
unexplored consequences - in LIBLO_METHOD_TIMESTAMP was set
but never (and rightly as not enough at any rate) used:

-            mtime = fs::last_write_time(parentGame.PluginsFolder());
+            //mtime = fs::last_write_time(parentGame.PluginsFolder()); // ut - only used in HasChanged()
+            // which for LIBLO_METHOD_TIMESTAMP returns true

I added mtime_data_dir and I check both in LoadOrder::HasChanged() - still
as noted in #12 this won't for instance reload the load order in case
of an esm/p flip (Bash's __fix_load_order will take care of that though)

Final touch for wrye-bash/wrye-bash#195 ?
@Ortham
Copy link
Owner Author

Ortham commented Dec 3, 2015

This issue has been greatly mitigated by 4cc5a5f, which loads only plugin headers when validating. As such, there's no pressing need to make the changes suggested here, but the suggestions remain valid.

@Ortham
Copy link
Owner Author

Ortham commented Dec 4, 2015

OK, actually got around to doing some profiling, and the results show that loading data from scratch typically takes 50 ms with 268 plugins installed and 255 of them active. With 13 plugins and none active, it takes < 1 ms, so there's a clear correlation to number of plugins.

This is on a VirtualBox virtual machine running on top of a Core i5-4300U with an SSD, so it's not a particularly fast environment anyway.

An initial implementation of re-using data where possible according to point 2 in the OP (not including inside the loading from file and loading active plugins functions) speeds up subsequent loads so that 1000 loads with no changes between them take only 4 times as long as a single load. As such, I think the changes are worth making.

@Ortham
Copy link
Owner Author

Ortham commented Dec 4, 2015

More profiling, and combining the isValid() checks with the Plugin constructors doesn't lead to any performance improvement, so I'm striking that from the OP.

@Ortham Ortham closed this as completed in 75f0751 Dec 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants