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

Save server row index instead of finding it #1716

Conversation

ecastro98
Copy link
Member

Saving/Caching the row index of each server in server list allow us to avoid making unnecesary for-loop (for each server) to find if the row already exists. For example on the first load, where the first action is just to add (not update nor remove), avoiding this will increase the performance a lot and it won't get so much time to load.

@StrixG StrixG added the enhancement New feature or request label Oct 15, 2020
@StrixG StrixG added this to the 1.6 milestone Oct 15, 2020
@Woovie
Copy link
Contributor

Woovie commented Oct 15, 2020

Thank you so much for this PR. I was trying to figure out where the caching opportunity was here on my own but my unfamiliarity with c++ really hindered me.

Copy link
Contributor

@qaisjp qaisjp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so gooood! It feels like a 100x browser load speedup! Amazing.

I left a tiny nit, but other than that, it looks great :)

Client/core/ServerBrowser/CServerBrowser.cpp Outdated Show resolved Hide resolved
Client/core/ServerBrowser/CServerBrowser.cpp Outdated Show resolved Hide resolved
Client/core/ServerBrowser/CServerBrowser.cpp Outdated Show resolved Hide resolved
Client/core/ServerBrowser/CServerList.h Show resolved Hide resolved
@Woovie Woovie self-requested a review October 15, 2020 20:55
Copy link
Contributor

@Woovie Woovie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once the changes requested by qaisjp are done, we will merge.

Co-authored-by: Qais Patankar <qaisjp@gmail.com>
@ecastro98 ecastro98 requested a review from Woovie October 15, 2020 23:53
@Woovie Woovie merged commit a708e68 into multitheftauto:master Oct 16, 2020
@ecastro98 ecastro98 deleted the refactor/server-browser-little-improvement branch April 9, 2023 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants