Subj -
Details
Details
- Reviewers
wraitii vladislavbelov
- Test that sending reports still work
- Test that profiler2 no longer shows a thread that's mostly useless
- Agree that we don't have to trigger sending on creation, and that waiting a small timeout is acceptable.
Diff Detail
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Lint Skipped - Unit
Unit Tests Skipped - Build Status
Buildable 19126 Build 45564: Vulcan Build Jenkins Build 45563: Vulcan Build (macOS) Jenkins Build 45562: Vulcan Build (Windows) Jenkins
Event Timeline
Comment Actions
Build failure - The Moirai have given mortals hearts that can endure.
builderr-debug-macos.txt
In file included from ../../../source/simulation2/components/CCmpParticleManager.cpp:20:
In file included from ../../../source/simulation2/system/Component.h:25:
In file included from ../../../source/simulation2/system/ComponentManager.h:22:
In file included from ../../../source/scriptinterface/ScriptInterface.h:22:
In file included from ../../../source/scriptinterface/ScriptConversions.h:21:
/Users/wfg/Jenkins/workspace/macos-differential/build/workspaces/gcc/../../../source/scriptinterface/ScriptRequest.h:32:10: fatal error: 'js/RootingAPI.h' file not found
#include "js/RootingAPI.h"
^~~~~~~~~~~~~~~~~
1 error generated.
make[1]: *** [obj/simulation2_Debug/CCmpParticleManager.o] Error 1
make: *** [simulation2] Error 2Link to build: https://jenkins.wildfiregames.com/job/macos-differential/5465/display/redirect
Comment Actions
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/6564/display/redirect
Comment Actions
I did not test that sending reports still work but i have saw Report Submission on thread 2. So i think it works.
GetShutdown on L257 should be done bevore the task is created(appended).
More things should be changed: code-coments thread -> task and condition-variable-> ?. for code changes i wrote inline coments.
Why is the Report sent repetatly and not only on startup and on enabling it? I don't think the hardware or the os changes while the game runs. (i definitly miss something)
Note for an future diff: one mutex should not be used to protect multiple variables. i.e: The mutex for the queue is locked only because we check if shutdown is set. some variables could propably be changed to atomics.
| source/ps/UserReport.cpp | ||
|---|---|---|
| 175 | assignement has no effect if they have the same value. (if statement not needed) | |
| 210 | pointless scope | |
| 251 | This look was only used for the condition variable. So is not required anymore. | |
| 273 | could be combined with the statement in the while condition. | |
Comment Actions
Thanks for the comments. Will try to fix them!
As to why it was send periodically I think the goal was probably to eventually send more data, like replays or crashlogs.
Comment Actions
The Future should be stored in the CUserReporterWorker or returned to main. Else the task is immediately canceled. That Future can then also be used to check if a update is already running (only push a task when the Future IsReady).
Comment Actions
As described in #5874 adapting the task to the task manager means esentialy removing the "waits". In case of the user reporter there are: Waiting on the condition_variable: This diff does reduse the use of the condition_variable to a negligible [leftover]. (leftover isn't connoted negative in this case)
Waiting on the network:
From https://curl.se/libcurl/c/curl_easy_perform.html
curl_easy_perform performs the entire request in a blocking manner and returns when done, or earlier if it fails. For non-blocking behavior, see curl_multi_perform.
We should switch to curl_multi_perform before we move the task to the TaskManager.
When we have repeating tasks (see #5876) we can repeatetly call curl_multi_perform.
When there is a connection proble The user reporter tries to reconnect after some time. That would also be a perfect fit for repeating tasks.