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

GRPC Blocking Thread #92

Merged
merged 18 commits into from
Mar 25, 2020
Merged

GRPC Blocking Thread #92

merged 18 commits into from
Mar 25, 2020

Conversation

RobertBColton
Copy link
Contributor

@RobertBColton RobertBColton commented Mar 7, 2020

Alright, so the idea behind this pull request is to throw out the QTimer that was driving the GRPC plugin (and causing high CPU usage). Its existence was largely due to the poor integration GRPC has with foreign event loops which I discussed over in #89

Luckily this is something the GRPC team is working on.
grpc/grpc#14796
grpc/grpc#20194
And it's expected to land in the GRPC Core in 2-3 weeks.
grpc/grpc#14565

So this pull request replaces the timer we were using with a blocking thread that interrupts and wakes up the main GUI thread when a GRPC event comes in for the client. Rusky pushed me to do it and it is what the GRPC devs suggest as a workaround in the above issues. It uses the magic of QMetaObject to pack the event parameters up in a nice QMetaEvent which later gets delivered to CompilerClient::UpdateLoop where the event is free to update the GUI safely back on the main event loop.

Fix the file pattern to whitelist only emake and emake.exe so it stops
picking up emake-tests.exe as the server. Set a timer delay larger than
0 so the server plugin stops hogging the CPU.
Use a blocking thread to poll for GRPC events and then wake up the Qt
main event loop for processing them using queued connections.
@RobertBColton RobertBColton merged commit fc01197 into master Mar 25, 2020
@RobertBColton RobertBColton deleted the grpc-blocking-thread2 branch March 25, 2020 23:54
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