DSP: Eliminate most global state #9348
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lioncash
force-pushed
the
dsp-deglobal
branch
3 times, most recently
from
5caffaf
to
070b4bc
Compare
smurf3tte
reviewed
Dec 21, 2020
smurf3tte
reviewed
Dec 21, 2020
lioncash
force-pushed
the
dsp-deglobal
branch
from
070b4bc
to
aedab6e
Compare
smurf3tte
reviewed
Dec 22, 2020
iwubcode
reviewed
Dec 22, 2020
|
Still need another pass but this looks really good! |
lioncash
force-pushed
the
dsp-deglobal
branch
from
aedab6e
to
55f2dbf
Compare
iwubcode
reviewed
Dec 22, 2020
iwubcode
reviewed
Dec 22, 2020
lioncash
force-pushed
the
dsp-deglobal
branch
from
55f2dbf
to
6aa5bb5
Compare
lioncash
force-pushed
the
dsp-deglobal
branch
2 times, most recently
from
847150d
to
f5c783d
Compare
lioncash
force-pushed
the
dsp-deglobal
branch
2 times, most recently
from
44e443a
to
8d4489b
Compare
An unfortunately large single commit that deglobalizes the DSP code.
(which I'm very sorry about).
This would have otherwise been extremely difficult to separate due to
extensive use of the globals in very coupling ways that would result in
more scaffolding to work around than is worth it.
Aside from the video code, I believe only the DSP code is the hairiest
to deal with in terms of globals, so I guess it's best to get this dealt
with right off the bat.
A summary of what this commit does:
- Turns the DSPInterpreter into its own class
This is the most involved portion of this change.
The bulk of the changes are turning non-member functions into member
functions that would be situated into the Interpreter class.
- Eliminates all usages to globals within DSPCore.
This generally involves turning a lot of non-member functions into
member functions that are either situated within SDSP or DSPCore.
- Discards DSPDebugInterface (it wasn't hooked up to anything,
and for the sake of eliminating global state, I'd rather get rid of
it than think up ways for this class to be integrated with
everything else.
- Readjusts the DSP JIT to handle calling out to member functions.
In most cases, this just means wrapping respective member function
calles into thunk functions.
Surprisingly, this doesn't even make use of the introduced System class.
It was possible all along to do this without it. We can house everything
within the DSPLLE class, which is quite nice =)
Localizes code that modifies m_dsp into the struct itself. This reduces the overal coupling between DSPCore and SDSP by reducing access to its member variables. This commit is only code movement and has no functional changes.
lioncash
force-pushed
the
dsp-deglobal
branch
from
70fd8e9
to
5f65bad
Compare
|
Are there any more reviews to be done? Last review was 6 days ago |
BhaaLseN
approved these changes
Dec 28, 2020
There was a problem hiding this comment.
Some files were difficult to review because of method reordering (which individually are just renames/relocations to match the new structure), so I kinda avoided this PR in the past days, but here we go. Code LGTM, untested. But I think we'll get some quick feedback once this is out in the wild in case something is really off.
leoetlino
approved these changes
Dec 28, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
An unfortunately large single commit that deglobalizes the DSP code. (which I'm very sorry about).
This would have otherwise been extremely difficult to separate due to extensive use of the globals in very coupling ways that would result in more scaffolding to work around than is worth it.
Aside from the video code, I believe only the DSP code is the hairiest to deal with in terms of globals, so I guess it's best to get this dealt with right off the bat.
A summary of what this commit does:
Turns the DSPInterpreter into its own class
This is the most involved portion of this change. The bulk of the changes are turning non-member functions into member
functions that would be situated into the Interpreter class.
Eliminates all usages of globals within DSPCore.
This generally involves turning a lot of non-member functions into member functions that are either situated within SDSP or DSPCore.
Discards DSPDebugInterface (it wasn't hooked up to anything, and for the sake of eliminating global state, I'd rather get rid of it than think up ways for this class to be integrated with everything else.
This hasn't been used since the migration to Qt.
Readjusts the DSP JIT to handle calling out to member functions.
In most cases, this just means wrapping respective member function calls into thunk functions.
Surprisingly, this doesn't even make use of the introduced System class. It was possible all along to do this without it. We can house everything within the DSPLLE class, which is quite nice =)