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

Avoid duplicated reading RPN code execution across multiple clients #27

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

cpuwolf
Copy link
Contributor

@cpuwolf cpuwolf commented Mar 15, 2024

@Koseng

While multiple clients Add(MF.SimVars.Add.xxxxRPNcodexxx) the exact same SimVars to WASM,
From MSFS2020 perspective, it is unnecessary to execute the same RPNcode several times in each frame. This is an enhancement change to reduce FPS impact
WASM will smartly execute duplicated SimVars only once in each frame.

introduce new data structure:

//RPN code execution for reading values in every frame
struct ReadRPNCode {
	std::string Code;
	int RetType; //RetType: 0:float 1:integer 2:string
	std::vector<SimVar> SimVars;
	std::vector<StringSimVar> StringSimVars;
};

‎wasm ‎1

@cpuwolf cpuwolf changed the title Avoid duplicated RPN code execuation across multiple clients Avoid duplicated reading RPN code execuation across multiple clients Mar 15, 2024
cpuwolf added 2 commits March 16, 2024 06:51
found below log shows NAV1 actually is no changing at all

 MobiFlight[Client_328838310]: SimVar (A:NAV ACTIVE FREQUENCY:1,MHz) with ID 21056 has value 108.4
 MobiFlight[Client_328838310]: SimVar (A:NAV ACTIVE FREQUENCY:1,MHz) with ID 21056 has value 108.4
 MobiFlight[Client_328838310]: SimVar (A:NAV ACTIVE FREQUENCY:1,MHz) with ID 21056 has value 108.4
 MobiFlight[Client_328838310]: SimVar (A:NAV ACTIVE FREQUENCY:1,MHz) with ID 21056 has value 108.4
 MobiFlight[Client_328838310]: SimVar (A:NAV ACTIVE FREQUENCY:1,MHz) with ID 21056 has value 108.4
 MobiFlight[Client_328838310]: SimVar (A:NAV ACTIVE FREQUENCY:1,MHz) with ID 21056 has value 108.4
 MobiFlight[Client_328838310]: SimVar (A:NAV ACTIVE FREQUENCY:1,MHz) with ID 21056 has value 108.4
 MobiFlight[Client_328838310]: SimVar (A:NAV ACTIVE FREQUENCY:1,MHz) with ID 21056 has value 108.4
 MobiFlight[Client_328838310]: SimVar (A:NAV ACTIVE FREQUENCY:1,MHz) with ID 21056 has value 108.4
 MobiFlight[Client_328838310]: SimVar (A:NAV ACTIVE FREQUENCY:1,MHz) with ID 21056 has value 108.4
 MobiFlight[Client_328838310]: SimVar (A:NAV ACTIVE FREQUENCY:1,MHz) with ID 21056 has value 108.4
 MobiFlight[Client_328838310]: SimVar (A:NAV ACTIVE FREQUENCY:1,MHz) with ID 21056 has value 108.4
 MobiFlight[Client_328838310]: SimVar (A:NAV ACTIVE FREQUENCY:1,MHz) with ID 21056 has value 108.4
 MobiFlight[Client_328838310]: SimVar (A:NAV ACTIVE FREQUENCY:1,MHz) with ID 21056 has value 108.4
@cpuwolf cpuwolf force-pushed the 1.0.1-client.dup.simvar branch from ac04712 to 152c509 Compare March 15, 2024 22:55
@DocMoebiuz
Copy link
Collaborator

i am currently at FSWeekend, i will be looking at this PR next week

@cpuwolf cpuwolf changed the title Avoid duplicated reading RPN code execuation across multiple clients Avoid duplicated reading RPN code execution across multiple clients Mar 26, 2024
@cpuwolf
Copy link
Contributor Author

cpuwolf commented May 5, 2024

@DocMoebiuz Could you take a look?

@DocMoebiuz
Copy link
Collaborator

Not yet tbh :(

@DocMoebiuz
Copy link
Collaborator

DocMoebiuz commented May 5, 2024

Does this work with strings too? yeah, looks like you are also treating strings.

How much of an impact is this? What did you see in FPS drop?

@cpuwolf
Copy link
Contributor Author

cpuwolf commented May 5, 2024

std::vector SimVars;
std::vector StringSimVars;

so string is considered in this change. I don't have direct evidence to see FPS drop, but this change will improve FPS when we have multiple clients

@DocMoebiuz think about you have multiple clients, but they have exact same simvars (L:N_FCU_HEADING), then WASM will execute "(L:N_FCU_HEADING)" multiple times in MSFS one frame update, it is totally waste of FPS

Copy link
Collaborator

@DocMoebiuz DocMoebiuz left a comment

Choose a reason for hiding this comment

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

Sorry for the tardy review.

src/Sources/Code/Module.cpp Outdated Show resolved Hide resolved
src/Sources/Code/Module.cpp Outdated Show resolved Hide resolved
src/Sources/Code/Module.cpp Outdated Show resolved Hide resolved
src/Sources/Code/Module.cpp Show resolved Hide resolved
src/Sources/Code/Module.cpp Show resolved Hide resolved
// data struct for dynamically registered SimVars
struct SimVar {
int ID;
int Offset;
std::string Name;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why remove? This was valuable in all the different log messages for troubleshooting. Was it not?

Copy link
Contributor Author

@cpuwolf cpuwolf Sep 5, 2024

Choose a reason for hiding this comment

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

I have a dedicated commit "relocate struct SimVar->Name to ReadRPNCode->Code"

the purpose of this pull is to avoid duplicated RPN code, the "Name" here actually is is RPN code, so we need to remove redundant "Name"

Copy link
Collaborator

Choose a reason for hiding this comment

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

But it is missing now in the log statements which is unfortunate.

src/Sources/Code/Module.cpp Outdated Show resolved Hide resolved

#if _DEBUG
std::cout << "MobiFlight[" << client->Name.c_str() << "]: SimVar " << simVar.Name.c_str();
std::cout << " with ID " << simVar.ID << " has value " << simVar.Value << std::endl;
std::cout << "MobiFlight[" << simVar.clint->Name.c_str() << "]: SimVar " << rpn.Code.c_str();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
std::cout << "MobiFlight[" << simVar.clint->Name.c_str() << "]: SimVar " << rpn.Code.c_str();
std::cout << "MobiFlight[" << simVar.client->Name.c_str() << "]: SimVar " << rpn.Code.c_str();

if (simVar.Value == stringVal) continue;
simVar.Value = stringVal;

WriteSimVar(simVar, simVar.clint);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
WriteSimVar(simVar, simVar.clint);
WriteSimVar(simVar, simVar.client);


#if _DEBUG
std::cout << "MobiFlight[" << client->Name.c_str() << "]: StringSimVar " << simVar.Name.c_str();
std::cout << " with ID " << simVar.ID << " has value " << simVar.Value << std::endl;
std::cout << "MobiFlight[" << simVar.clint->Name.c_str() << "]: StringSimVar " << rpn.Code.c_str();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
std::cout << "MobiFlight[" << simVar.clint->Name.c_str() << "]: StringSimVar " << rpn.Code.c_str();
std::cout << "MobiFlight[" << simVar.client->Name.c_str() << "]: StringSimVar " << rpn.Code.c_str();

@DocMoebiuz
Copy link
Collaborator

@cpuwolf you marked some of the comments as resolved but you might not have pushed your changes yet.

@cpuwolf
Copy link
Contributor Author

cpuwolf commented Sep 7, 2024

@cpuwolf you marked some of the comments as resolved but you might not have pushed your changes yet.

should I resubmit the pull? I am expecting Github can do online review and revise the code realtime

@DocMoebiuz
Copy link
Collaborator

You have to make the changes in your branch and simply push the branch again. The PR will automatically update then. But simply resolving the conversation doesn't do it.

@cpuwolf
Copy link
Contributor Author

cpuwolf commented Oct 23, 2024

@DocMoebiuz I have uploaded revised version

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.

2 participants