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

[Discussion] LocalizationTable Background Thread? #1768

Open
BraedonWooding opened this issue Jan 30, 2017 · 10 comments
Open

[Discussion] LocalizationTable Background Thread? #1768

BraedonWooding opened this issue Jan 30, 2017 · 10 comments

Comments

@BraedonWooding
Copy link
Collaborator

Now I know there is a PR on the downloader aspect but what i've found tricky when trying to improve the speed of things like loading/saving of the settings menu is the biggest speed problem is the ...LocalizationTable.SetLocalization(int) where it seems to take quite a considerable amount of time. I guess it's more of a discussion about its speed then anything else. It also has a few weird occurrences and some weird here and there bits. So what do we think?

@BraedonWooding BraedonWooding changed the title LocalizationTable Background Thread? [Discussion] LocalizationTable Background Thread? Jan 30, 2017
@NogginBops
Copy link
Contributor

Does the localization table load in all localization data? That would slow down lookup considerably, if we only had like the last few languages loaded or like the current language + English I suspect it would be a lot faster.

@BraedonWooding
Copy link
Collaborator Author

The localization table speed is dependent partly on the total languages from what I can see (it's a bit from x() to y() back to x() then to z() back to y()... so it's a little hard to follow if just quickly glancing), but what seems to be the problem is a cumulative effect for example it uses a GameObject.Find and GetComponent every time to get the localization loader... and it then seems to load all the localization of every file but just 'skips' over the ones that aren't the default language but it does maybe 80% of the processing so basically it seems it presumes every language file contains multiple languages or something like that it's a little weird... So I think that a background thread (and some cleanup) would more hide this processing rather than fix it? Though I don't really think any overhaul is a top priority right now which is why maybe a background thread could be useful.

@bjubes
Copy link
Collaborator

bjubes commented Jan 31, 2017

@BraedonWooding
Copy link
Collaborator Author

BraedonWooding commented Jan 31, 2017 via email

@bjubes
Copy link
Collaborator

bjubes commented Jan 31, 2017

my bad. When you change the language and then save changes, that delay becomes evident.
capture

however its worth noting that a lot of the delays seem to be ultimately due to log calls. I can't extend the image far enough to see the rest, but its possible a good percentage of the delay can be log spam

@bjubes
Copy link
Collaborator

bjubes commented Jan 31, 2017

Im not getting a lot of baseline lag so I can't tell how well this works or not, but try disabling debug logs by commeting out line 72 of localization loader

UnityDebugger.Debugger.Log("LocalizationLoader", "Loaded localization at path: " + file);

This reduced the time (again its hard to tell) for me, and the profiler now just shows that all the lag is comming from LogStringToConsole, with only 10% going to LoadLocalizationInDirectory

@BraedonWooding
Copy link
Collaborator Author

Ohh I see, there is also the LoadingLanguagesFinished? or is that where that line is coming from

@bjubes
Copy link
Collaborator

bjubes commented Feb 5, 2017

LoadingLanguagesFinished calls a callback CBLocalizationFilesChanged, so I don't think the profiler can track down what is actually being called, most of it is just has Action.Invoke_void_this all the way down the screen. maybe thats something else to look into, but if its only 167ms on other peoples machines as well then its not really worth optimizing.

This does however bring up the bigger point that log calls are causing performance issues even when they are toggled off and even when the game is built and not logging anything.

@koosemose
Copy link
Collaborator

koosemose commented Feb 10, 2017

Are you sure they're causing an issue when toggled off? With all channels toggled off, I only get the call to Debugger.Log taking .1% of the time in LoadLocalizationInDirectory (.33ms out of 20.41) in deep profiling, and I can't even see any noticeable effects when not deep profiling. And it should be even less in builds, though since it's such a small amount it would be pretty negligible in the scheme of things, since it should short circuit even sooner.

The most significant slowdowns I see are LoadLocalizationFile and LoadConfigFile in LocalizationTable, with most of the time in those of course being the actual ReadAllLines call.

With those being the source of the slowdowns, I'm not sure how well it would work to do in the background, I assume we would still need to halt gameplay, since most of it is reading in the strings that are used everywhere, so I'm not sure how well things would work if we're changing the strings in another thread, it seems that might cause some issues.

@BraedonWooding
Copy link
Collaborator Author

Yes I agree with those findings of slow downs. Yes that is a fair point, I actually feel it's fine and just kind of one of those things that will never really be a big problem but will just sit there. Honestly I'm hard pressed to find a game with localization that doesn't take a considerable (a second or two) amount of time when saving and even quite a few that don't have it take a decent amount of time. So I think we are fine in this case :D, I'll close the issue because it seems to be the consensus that it doesn't effect gameplay enough and the exact cause is more ambiguous.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants