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

Cumulative optimizations #336

Merged
merged 6 commits into from
Apr 4, 2021

Conversation

indigodarkwolf
Copy link
Collaborator

A collection of optimizations ported over from my C++-based fork of the X16 emulator:

  • Rewrote joystick implementation (this supersedes the PR at NES gamepads are dead, long live SNES gamepads! #335):
    • Joysticks can now bind at run-time, because I have a wireless controller and can never, ever remember to turn it on before launching the emulator.
    • Joysticks behavior is now event-driven instead of requiring step function every CPU cycle and extra state polling separate from the SDL event system.
  • Rewrote PS2 implementation:
    • Significantly less branching
    • Now only updates immediately before reads and write to the appropriate VIA ports. Big performance win.
  • Performance tweaks to VERA implementation:
    • A trivial API change was staring us in the face, allowing us to reduce the calls to video_step() from one per CPU cycle to one per CPU instruction. Also a significant win.
    • Another trivial API change of the same nature for vera_spi_step().

Altogether, warp speed on a Raspberri Pi4 has gone from 101% to 157%.
Non-warp speed on a Raspberri Pi4 has gone from 80% to a rock-steady 100%.

I'm certain my previous experiments with RPi4 pegged performance in the mid-30% range, and I cannot explain the discrepancy in baseline performance between then and now. Go team? Compiler optimizations added at some point? RPi4 update made more resources available? Whatever the case, this still represents a significant jump in performance by removing the most significant bottlenecks outside of the hot paths of the VERA and base CPU emulation, and all platforms will benefit from this.

ps2.c Outdated
void
mouse_button_down(int num)
{
buttons |= 1 << num;
mouse_send_state();

Choose a reason for hiding this comment

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

Pull request #321 removed those function calls, for a good reason. Why did you add them back?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's essentially a merge error since I completely rewrote ps2.c/ps2.h on my fork. Will fix.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should be cleaned up now. Sorry about that. And the emulator should be even faster still as a result. 😃

@indigodarkwolf
Copy link
Collaborator Author

Changes resolved with latest submits to master.

@indigodarkwolf
Copy link
Collaborator Author

With the latest i2c stuff checked in, I'm seeing 76% -> barely eeking out 100% on an RPi4. I'll be looking at the i2c stuff eventually, I'm sure, to see if I can't give it the same treatment as the ps2 emulation. But not for this PR, and probably not for at least a week.

Copy link
Collaborator

@mist64 mist64 left a comment

Choose a reason for hiding this comment

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

looks good, just some nits

void mouse_move(int x, int y);
void mouse_send_state();
// fake mouse
void mouse_button_down(int num);
Copy link
Collaborator

Choose a reason for hiding this comment

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

please no indentation to line up function names

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will try to keep this in mind in the future. I blame clang-format.

@@ -18,12 +19,19 @@
// VIA#1
//
// PA0: PS2KDAT PS/2 DATA keyboard
// PB0-2 ROM bank
Copy link
Collaborator

Choose a reason for hiding this comment

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

where do these outdated lines come from? :)

@@ -37,59 +45,55 @@

static uint8_t via1registers[16];

void
via1_init()
void via1_init()
Copy link
Collaborator

Choose a reason for hiding this comment

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

please keep the type in a separate line, like in all other code

{
uint8_t out_mode = reg_composer[0] & 3;

bool new_frame = false;
float advance = ((out_mode & 2) ? NTSC_PIXEL_FREQ : VGA_PIXEL_FREQ) / mhz;
float advance = ((out_mode & 2) ? NTSC_PIXEL_FREQ : VGA_PIXEL_FREQ) * steps / mhz;
Copy link
Collaborator

Choose a reason for hiding this comment

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

no identation of this kind, please. it will keep diffs minimal.

@mist64 mist64 merged commit 9ebe9df into commanderx16:master Apr 4, 2021
@mist64
Copy link
Collaborator

mist64 commented Apr 4, 2021

Never mind, I merged it, and I'll change the nits myself :)

mist64 pushed a commit that referenced this pull request Apr 4, 2021
* Removing branches and needless loops from ps2, joystick, video_step, vera_spi_step.
* byte -> value: Variable name change: Windows SDK conflict.
* Documentation updates
@mist64
Copy link
Collaborator

mist64 commented Apr 4, 2021

amended by changes the commit, new commit: abfe032

mist64 pushed a commit that referenced this pull request Apr 4, 2021
* Removing branches and needless loops from ps2, joystick, video_step, vera_spi_step.
* byte -> value: Variable name change: Windows SDK conflict.
* Documentation updates
@indigodarkwolf
Copy link
Collaborator Author

Awesome, thanks!

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