kosarev / z80

Fast and flexible Z80/i8080 emulator with C++ and Python APIs
MIT License
63 stars 10 forks source link

Mark overrideable functions as virtual #48

Closed dargueta closed 2 years ago

dargueta commented 2 years ago

What

Why

clang-tidy keeps yelling at me that my subclass is shadowing non-overrideable functions in the superclass. This also prevents us from using the override keyword to guard against typos and interface changes. By marking these functions as virtual

arpruss commented 2 years ago

Does this have an impact on performance?

dargueta commented 2 years ago

That is an excellent question. I'll try to do some performance testing and get back to you. Common wisdom says marking a function as virtual hurts performance, but from what I've read that's one of those "that used to be the case" *, like driving around after you get a jump will recharge your battery or drinking moonshine will make you go blind.

Only way to know for sure is to test.


* To be specific, cache misses loading the vtable. Modern CPUs have cache sizes well into the megabyte range.

kosarev commented 2 years ago

that's one of those "that used to be the case" *

Hi Diego, thanks for the patch. Relying on compile-time polymorphism is one of the most basic features of this implementation, enabling zero performance overhead. When necessary, it should be fairly trivial to employ virtual calls on top of the current approach whereas the opposite would be very much impossible. Regarding the clang-tidy warnings, would suppressing them be an option?

arpruss commented 2 years ago

Even if on the latest modern desktop and mobile systems there is no significant loss of performance, I can imagine use cases for this emulator on less powerful systems, like microcontrollers. My USB library for Arduino on stm32f1 initially used virtuals, but this resulted in significantly higher RAM usage (which mattered, because the microcontroller I was using has 20kb RAM and 128kb flash).

dargueta commented 2 years ago

less powerful systems, like microcontrollers

Ah. I didn't think of that, sorry.

Regarding the clang-tidy warnings, would suppressing them be an option?

Presumably, if I can find my IDE's settings for it.

Anyway, I've been convinced this is a bad idea and will close it. Thanks for the feedback!