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

Added a benchmark for game logic #10281

Closed
wants to merge 3 commits into from
Closed

Conversation

frutiemax
Copy link
Contributor

Google benchmark parameters seem to work. Still need to figure out what to do with CPU iterations. The command to be called is openrct2 benchlogic xxx.sv6


using namespace OpenRCT2;

static void BM_gamelogic(benchmark::State& state, IContext* context)
Copy link
Member

Choose a reason for hiding this comment

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

Why can't it use the standard form

for (auto _ : state)
{
    context->GetGameState()->UpdateLogic();
}

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, you found the problem with my code, you actually need this. Why?

Copy link
Contributor

Choose a reason for hiding this comment

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

The loop is actually measuring the time, you don't need your timer. I'm just not sure it is safe to call context->GetGameState()->UpdateLogic() repeatedly in a loop.

@janisozaur
Copy link
Member

Other than the comment I posted above:

  1. Make sure you formatted your code
  2. Make sure your commit message conforms to https://github.com/OpenRCT2/OpenRCT2/wiki/Commit-Messages

The xcode failure needs to be handled by someone with access to xcode. Ping @AaronVanGeffen @Gymnasiast

@janisozaur
Copy link
Member

That's not what I meant. I think in current form it will cause problems because you enabled manual timing and provide your own measurements, while all this should be handled by state. Why is it necessary to do your own timing instead of using the benchmark's one?

@frutiemax
Copy link
Contributor Author

It is not necessary to do manual timing. But it works nonetheless and it is one of the examples in the documentation of the Google Benchmark library.

@janisozaur
Copy link
Member

Since it is not necessary, I would prefer to use the simpler invocation

frutiemax92 and others added 3 commits March 1, 2020 17:20
@AaronVanGeffen
Copy link
Member

@frutiemax I've rebased this PR and pushed a fix for the Xcode project. Do you intend to continue to work on this?


using namespace OpenRCT2;

static void BM_gamelogic(benchmark::State& state, IContext* context)
Copy link
Contributor

Choose a reason for hiding this comment

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

The loop is actually measuring the time, you don't need your timer. I'm just not sure it is safe to call context->GetGameState()->UpdateLogic() repeatedly in a loop.


using namespace OpenRCT2;

static void BM_gamelogic(benchmark::State& state, IContext* context)
Copy link
Contributor

Choose a reason for hiding this comment

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

You also don't need to call the function with BM_ prefix.

@AaronVanGeffen
Copy link
Member

This is essentially doing the same thing as #10700. We need to decide which to merge.

@tupaschoal tupaschoal added this to the v0.2.6 - Consideration milestone Apr 13, 2020
@tupaschoal tupaschoal modified the milestones: v0.2.6 - Consideration, v0.2.7 Consideration Apr 17, 2020
@Gymnasiast Gymnasiast marked this pull request as draft July 9, 2020 13:04
@frutiemax frutiemax closed this Jul 24, 2020
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.

7 participants