Improve tradeships.lua #2851
Improve tradeships.lua #2851
Conversation
… volumes of space to analyse. Reduce the maximum and ensure the minimum distances requested of GetNearbySystems so no more requesting 67.7 light years worth of systems! Also start with the half-distance and use it if it is acceptable, only go further if it's not.
…ath by ordering them nearest to furthest when we fill the cache.
…e more profile logging in the GasGiant code.
|
Why do you think that the order in which the sector paths are passed to the cache feeder make any difference? The cache filling should happen when in hyperspace anyway. Or are we talking about when the game is just started? |
|
Game start, although since |
Not really, as it only generates sectors not already in the cache. And the generation at the end of hyperspacing is triggered with the same destination system as at the start of hyperspacing and thus should not generate any new sectors. Edit: This is quite subtle, though. It works, because the new |
|
This is though a minor part of the optimisation. I encountered it, found it lacking, made it make more sense - it does make sense to order things in the order that we'd like them - and in my subsequent runs it did give consistently lower numbers. So it's simple, logical, and gives a small benefit early on in the game (after starting or when loading) when we see the absolute worst stalls imaginable. So, you just don't like it? I'm confused. Is there an actual problem with doing it rather than just submitting all of the jobs in an arbitrary and unordered state? |
There could be since the sorting is done synchronously in the main thread (you cut the number of sectors generated by eight at the same time, is that related?). And actually one does not need a reason to keep the code simple. I was wondering if there is an actual problem that needed an ordered state, that's why I asked. Did really make the sorting any difference or was it the other changes (cutting range of Sector cache generation does not take long in my experience (or it least it did not, maybe some of the jobs now get stalled behind the gas giant jobs). |
|
Ah I see, no that's just how the commit order makes it look :) First I did a whole bunch of profiling and logging to determine the problem areas both in Once I'd found the biggest problem areas I did the re-ordering as almost the first thing thinking it would fix the startup time - it makes a small difference. Then I modified the Lua to cap the distances, attempt shorter ones first and a couple of other bits. Then I came back later and reduced the amount of StarSystems generated because I had capped the distance that could be queried in Lua so it didn't make sense to keep generating so much. As well as trying out a few other ideas, most of which didn't pan out. The commits were done in order of the importance of the effect they'd had, because I had already made all of the changes. It was either going to be one big commit, or 3 so that people can see what was changed more easily. It only doesn't take long on certain machines, the little Linux box I have is technically a quad core but it's an AMD so not really and it can take lot longer. Single and Dual Core PC's only get 1 thread for async processing so getting the correct data first should make a big difference for them, even on my quad-core I saw a small difference Besides it's just a is there any actual problem with the rest of the changes that break |
|
I mean this does make the worst stall around ~1000 milliseconds with most of the subsequent ones at less then ~0.8 milliseconds. That's compared to the ~51000 to ~64000 milliseconds that I was seeing followed by many ~7000 to ~2000 millisecond stalls. It virtually eliminates the stalling that we've been seeing from Do ships get stuck at star ports? Do they never leave, never arrive, do something weird like takeoff and hang around above starports because they can't jump etc? |
|
Sorry, had no time to look at You might want to cross-check with @walterar, though. I think he has also some changes in the pipeline, which I didn't follow closely. As for reducing the number of generated sectors. I thought the radius came from what was needed for the sector view, not so much for getting near star systems. But I may be mistaken. |
|
I thought @walterar had actually removed it from his mod entirely? No the number of sectors was massive overkill for just the sector view, it was set based on the kind of distance requests we were getting. |
|
Yes but @walterar doesn't appear to want to submit PR's for any of those things and they change a lot of stuff. If we eventually get rid of 'TradeShips.lua' that's fine but we can cross that bridge if he ever submits a PR. |
|
Freshly see this. Trafic.lua is a work in progress. The attentive chatter against my inspiration. :) |
This is entirely the fault of @robn for goading me into it ;) very clever...
First of all I profiled to find the worst offenders and added some logging. It turns out that we were requesting some MASSIVE distances in
GetNearbySystems. I think the largest I saw was 67.7 Light Years... that's not very NEAR in my book! I've limited it to a maximum of 30 Light years which still describes a sphere of space 60 light years across. I've also started using only the first half of the possible Hyperspace distance IF it's good enough.Next I've started sorting the list of SystemPaths that are submitted as jobs to be generated/cached so that we might get the ones closest to our origin system first. Rather than those furthest away being generated first.
Finally I noticed that we seem to need some extra profile logging in
GasGiant.cpp.Now when I profile...
GetNearbySystemsdoesn't even show up and there's very few stalls, certainly none on the order of 50 seconds like I was seeing when I started :)Now it's time for Tea!
Andy