descawed / tesconvert

A tool for converting player characters between The Elder Scrolls III: Morrowind and The Elder Scrolls IV: Oblivion
0 stars 0 forks source link

Optimize Oblivion plugin loading #14

Closed descawed closed 4 years ago

descawed commented 4 years ago

Loading Oblivion plugins is very slow and uses a lot of memory. Should profile this, but my guess is that the root of the issue is eagerly decompressing compressed fields. We should probably decompress on-demand. Also look into parallelizing plugin loading.

descawed commented 4 years ago

Implementing lazy decompression is actually proving to be extremely challenging, at least at my Rust skill level. There are two issues.

  1. Decompressing the data can fail. Currently, this happens in Tes4Record::load, which could already fail. On-demand decompression means that now any operation on fields can fail, such as the iter and len methods. Moreover, even if the data is decompressed successfully, loading fields from this data can fail. This means that the iterator methods, which currently return Box<dyn Iterator<Item = &F> + '_>, would actually need to become Result<Box<dyn Iterator<Item = Result<&F, TesError>> + '_>, TesError>. In addition to being a mess, this means records would no longer be compatible with IntoIterator.
  2. Decompression changes the vector of fields, which means any operation on fields now needs to be mutable. I've been trying to address this with interior mutability by keeping the compressed data and fields in a RefCell, but I haven't yet been able to figure out how to return (or create) an iterator which can return references to elements inside the RefCell due to lifetime issues with Ref/RefMut. This SO post provides some insight into how this might be accomplished, but it also means the trait would no longer be able to return an iterator; it would return a type whose references are iterators, complicating the interface.

Even worse is that the iteration methods are defined on the trait, so any mutability/fallibility changes would also have to be made to Tes3Record, even though that type doesn't need a mutable reference for iteration and can't fail while iterating. The alternative would be to dispense with the Record trait entirely, which would have downstream effects resulting in rewriting large parts of the application and duplicating a lot of code.

Another option that occurs to me would be to implement some sort of generic two-phase initialization system for records. Something like a prepare method you would call that would do the decompression if necessary and an is_prepared method indicating whether the record has been "prepared". I could also leverage this on the Morrowind side to do on-demand field parsing, obtaining some speed-up there. The obvious downside is that having to explicitly check if every record is prepared, and then prepare it if not, before using it is extremely cumbersome. Maybe I could implement some wrapper type that derefs to a record and then transparently prepare it when an operation is requested. But some operations should not require being prepared. For instance, I want to be able to obtain the record's form ID so I can create the form ID mapping without forcing the record to be fully parsed. I'm not sure how a wrapper type would enforce automatic preparation for some methods but not others. And even if this addresses the mutability/ownership/verbosity issues, it doesn't address the fallibility issue.

descawed commented 4 years ago

Lazy loading is addressed by PRs #15 and #16.

descawed commented 4 years ago

Performance is acceptable now. On my machine with vanilla Morrowind and Oblivion load orders and saves from near the start of the game, conversion takes about 10 seconds.