Enhancement: Fitting Calculation Speed

[Ebag333]

So now that we've (mostly) eliminated the start up issues, the next slow performing area is the fitting recalcs. Here's a list of pain points, in no particular order.

1. We run a full recalc on empty tabs. (Should be easy to check for an empty fit and then bail.)
2. The first load, we have to import all effects called. For an empty Gnosis in full debug mode this amounts to a good 5-6 second delay. (This number goes up as the fitting recalc complexity goes up.) We should be able to address this with the suggestion in #976.
3. The first time effects are called is slow. With the same empty Gnosis, time took 3-4 seconds. Subsequent recalcs are much faster taking .1 to .3 seconds. (This is separate from the import). We might be stuck with this.
4. There are exceptions when fitting calcs are being run. For example def fitSelected (in FitSpawner) stack traces the first recalc (but not future ones). If you dig into the objects, you will find stack traces on various properties. Most of these aren't caught/handled at all, which likely causes all sorts of problems.
5. We run recalcs 2-3 times every time the tiniest thing changes. Since they are full recalcs, this makes doing things such as adding 8 turrents or 10 drones super painful. This may be able to be addressed by moving the recalc engine into a threaded handler as per #976.

[blitzmann]

We run a full recalc on empty tabs.

Even if a fit is "empty", we still have character skills applying to the hull, implants being applied, etc. So these still need to be calculated, and I don't think we can just "bail". Additionally, if there aren't any modules, the calcs for that should just skip already. We still loop over them, but a simple loop over ~15-30 items is negligible.

There are exceptions when fitting calcs are being run. For example def fitSelected (in FitSpawner) stack traces the first recalc (but not future ones). If you dig into the objects, you will find stack traces on various properties. Most of these aren't caught/handled at all, which likely causes all sorts of problems.

I don't get exceptions. If you've found exceptions happening during fit calculation, please post this as a separate issue as that should probably be addressed.

We run recalcs 2-3 times every time the tiniest thing changes.

This shouldn't be true either. We run recalc a couple times when fit is first loaded, which is an issue but last I looked at it I determined nothing could be done at the time, but otherwise it's always run once per action, yes. I'd like to know if you're able to get a fit to calc multiple times after adding one module. :)

[Ebag333]

I don't get exceptions. If you've found exceptions happening during fit calculation, please post this as a separate issue as that should probably be addressed.

The exceptions are either hidden with something akin to:

try:
    # do code
except:
    pass

(which the logbook PR exposes a lot of, because now we log when we hit that exception for the most part.)

Or buried deep in the object. You wouldn't see those unless you were hunting for them. They seem to be generated when we're gathering properties on various things, so the stacktrace is seen in the object but there's not an easy way to extract out the stack trace (that I've seen). (Value of the property is the error passed, but not the stacktrace itself.)

This shouldn't be true either. We run recalc a couple times when fit is first loaded, which is an issue but last I looked at it I determined nothing could be done at the time, but otherwise it's always run once per action, yes. I'd like to know if you're able to get a fit to calc multiple times after adding one module. :)

This is true. We do trigger a recalc for each change,, so what users see as one action (add gun, load ammo) will cause 3 recalcs.

I should have been more clear, we're doing 2-3 recalcs per fit load, and 1 recalc for each change which stacks up quite quickly.

[blitzmann]

This is true.

No, it's not true. As I explained and you verified, we have 1 recalc per action, sans fit loading. As long as it's not doing 2-3 (which would be a big problem), this is normal behavior, albeit slow on some systems.

[blitzmann]

As for the try... except stuff, I'd need a specific example. But if it really is an error, then logging it isn't a bad idea

[Ebag333]

This is true.

No, it's not true.

I was saying your statement is true. :)

As for the try... except stuff, I'd need a specific example. But if it really is an error, then logging it isn't a bad idea

I gave an example in the original post. If you look at the logbook PR, you'll see where I added logging to the catch.

For example def fitSelected (in FitSpawner) stack traces the first recalc (but not future ones).

[blitzmann]

I was saying your statement is true. :)

Well that's confusing @_@

[Ebag333]

I don't get exceptions. If you've found exceptions happening during fit calculation, please post this as a separate issue as that should probably be addressed.

It's a separate issue, and fairly pervasive, but tracking down and tackling this will be a literally one by one type scenario.

It's going to be tons of fun too. :(

Here's an example of one: https://puu.sh/u1C8r/3484ce3080.png

I put the break further in, but you can break on line 722 and should see it. Selecting view doesn't give the full stacktrace, only 'NoneType' object is not iterable.

It makes sense why we're get it it (passing in nothing instead of a list of fits), and most of the rest are likely similar....but we still shouldn't do it. Also the bigger question is, why aren't we getting stacktraces written back out?

[blitzmann]

It makes sense why we're get it it (passing in nothing instead of a list of fits), and most of the rest are likely similar....but we still shouldn't do it.

In that particular example...

    def fits(self):
        for mod in self.modules:
            if not mod.fits(self):
                return False

        return True

Do we even use this code? I don't think so, it might just be that this is old code. We'll have to take a look and remove it if necessary.

As to the error we're getting, it's 'NoneType' object is not iterable which is thrown when trying to access modified attributes. Considering this is during fit calculation, and we run a clear() on everything to clear the previous modified values, it makes sense that this would error out because we just don't have modified attributes. I haven't delved farther into it, but I don't think this is really a problem considering we wouldn't normally call this during fit calculation.

In general, though, if we're getting stacktraces through pyCharms evaluations, that's more or less normal and a side effect of stopping execution in the middle of the program. We get the stacktraces because pyCharm is trying to evaluate them to show us the data they contain when in reality they don't have the data they need to perform properly. I've seen these first hand every now and then, but they don't happen during normal operation (we're not). I'm not sure we should be implementing a bunch of fixes to fix something on pyCharms end that doesn't really concern pyfa's normal operation.

[blitzmann]

Also: #859 doing something like this would be beneficial I think. I'll get a few example up, and then we can spend some time going through effects and applying it.

[blitzmann]

Closing this, I think it was determined that the stacktraces noted for the properties were a by-product of pycharm that doesn't happen during normal execution of code, so that's fine. As for the calc speed... most of the discussion revolved around threading it out, which is something that could be interesting to see. I worry about thread-unsafe operations happening during calcs, but that would be a discussion for when it's looked into. :D