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

Linked hash map map memory implementation comes with extreme cpu use #26372

Closed
l29ah opened this issue Oct 23, 2018 · 14 comments

Comments

Projects
5 participants
@l29ah
Copy link
Contributor

commented Oct 23, 2018

Describe the bug
9bfc9b3 is the first bad commit
It even makes running snow slow!

To Reproduce
Steps to reproduce the behavior:
Build the game.
Load an existing game.
Enjoy.

@cadyb

This comment has been minimized.

Copy link

commented Oct 23, 2018

I also noticed a very high cpu 97% usage when running 7a7bec0
It seemed to get worse the more tiles the character could see. Inside a house with the curtains drawn was seemingly fine but as soon as you went outside it would take seconds to move a single tile
Updating to d6c0138 doesn't solve the issue
Reverting back to 30a0822 the commit before bfc9b30febdb57ac98708c11f9da3486832a0a1 drops cpu usage down to 11%

All CPU %s where tested in an open field.

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Oct 24, 2018

Can you provide lagging savegame?

@kevingranade

This comment has been minimized.

Copy link
Member

commented Oct 24, 2018

Are you seeing this on tiles or curses?

@kevingranade

This comment has been minimized.

Copy link
Member

commented Oct 24, 2018

Thanks for reporting that it's happening when snow effects are displaying, that's a valuable clue.
I suspect what's happening here is that you have animation rate set relatively high, and the weather animation code calls map::drawsq() or the tiles equivalent to re-draw an individual map tile to "erase" the weather glyph or tile. That code then calls the memorize code at a very high rate, driving lots of CPU utilization. It didn't cause a problem before my change because there's a "finalize" method that would do the relatively expensive part of applying those tiles to the memory. I was hoping the new code made memorization cheap enough to not bother with that, and I didn't test this scenario with fast weather animation.

If that's correct, I can probably somewhat resurrect the batching/finalization method and eliminate this slowdown.

@l29ah

This comment has been minimized.

Copy link
Contributor Author

commented Oct 24, 2018

I'm on curses, and i didn't touch the animation settings.

@l29ah

This comment has been minimized.

Copy link
Contributor Author

commented Oct 24, 2018

@kevingranade kevingranade added this to Need Confirmation in 0.D Release via automation Oct 24, 2018

@kevingranade kevingranade added this to the 0.D milestone Oct 24, 2018

@kevingranade

This comment has been minimized.

Copy link
Member

commented Oct 25, 2018

After a peek at a profile the animation code, it's clear that at least a component of the problem is that it's calling game::draw_ter() to clear the map instead of doing targeted erasures like I thought it was.

This results in a huge number of useless calls to memorize_symbol() and unordered_map::find().

0.D Release automation moved this from Need Confirmation to Closed Issues Oct 25, 2018

@l29ah

This comment has been minimized.

Copy link
Contributor Author

commented Oct 25, 2018

The provided save file still demonstrates >10s per step outside of the car, but better than it used to be.
x86_64 Intel(R) Core(TM) i5-3320M CPU @ 2.60GHz GenuineIntel GNU/Linux

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Oct 25, 2018

Hmm... I cannot reproduce this - the game runs just fine. Though I tried on i7 CPU.

Did you download pre-compiled binary or compiled by yourself? Also have you tried tiles version?

@l29ah

This comment has been minimized.

Copy link
Contributor Author

commented Oct 25, 2018

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Oct 25, 2018

What was your compilation command line? Did you use RELEASE=1?

@l29ah

This comment has been minimized.

Copy link
Contributor Author

commented Oct 26, 2018

Just make. No, as i'm trying to catch #26291 meanwhile.

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Oct 26, 2018

Debug builds were always slow. It is currently terribly slow even for debug build and we should do something, but if release builds are okay that is not a priority.

Could you try compiling and running with RELEASE=1 to make sure it is playable on your system?

@l29ah

This comment has been minimized.

Copy link
Contributor Author

commented Oct 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.