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

performance optimization: first steps #307

Merged
merged 2 commits into from
Jun 10, 2022
Merged

performance optimization: first steps #307

merged 2 commits into from
Jun 10, 2022

Conversation

Kolcha
Copy link
Contributor

@Kolcha Kolcha commented Jun 9, 2022

these 2 commits improve just one function (see its description for details), but the result is impressive - the function became twice faster. and this noticeably affected the whole rendering.

before changes:
Screenshot_20220610_010132
after this changes:
Screenshot_20220610_010541

and this is what I noticed just looked over the sources... I will look more and likely find more similar prominent places that can be easily optimized. this is just the first step.

Kolcha added 2 commits June 10, 2022 00:38
BlockIdentifier::getBlockInfo() is one of (or even the most)
frequently called functions during map rendering, so it should be
fast as possible. but now it is not - it does search on map twice:
one for contains() call, and another one on operator[]. this can be
easily avoided, and this is obvious optimization.
this is further optimization of BlockIdentifier::getBlockInfo().
there is nothing that requires sorting / specific order, so having
map is not really necessary, so hash table will suit better, and
it has constant time for element access (what is crucial in there).
@mrkite mrkite merged commit 752c01e into mrkite:master Jun 10, 2022
@Kolcha Kolcha deleted the optimization branch June 10, 2022 05:39
@EtlamGit
Copy link
Collaborator

Yes, for larger number of elements a QHash should be much faster than a QMap. And not searching twice is obviously faster.

Some time ago we measured performance of different containers (#246).
The outcome was, that Qt containers are in some cases slower than C++ std:: variants.
And at many locations there is no strict need to use the Qt containers. It is just cleaner to have it less mixed.
But there might be some performance to gain from that also.

My personal experience is that I do not suffer from rendering performance. I always struggle with file-IO performance (world is stored on network drive). Therefore it is one of my goals to improve file reading, rendering speed was neglected during the last years.

@Kolcha
Copy link
Contributor Author

Kolcha commented Jun 10, 2022

first of all, I have no plans to achieve the maximum possible performance or smooth rendering (I'm also fine with delays), I just noticed the "bad place", and think such obvious things must be fixed/optimized, especially when it costs nothing (just a few lines must be changed with no consequences).

The outcome was, that Qt containers are in some cases slower than C++ std:: variants.
And at many locations there is no strict need to use the Qt containers. It is just cleaner to have it less mixed.

I completely agree, code is written in Qt - let's use Qt containers unless there is a strict reason do not so. and those Qt containers may be slower it is expected for me - Qt metaobject system definitely leaves some footprint on performance.

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

Successfully merging this pull request may close these issues.

3 participants