[core] Add USE_BUSY_WAITING build option #711
Conversation
maxsharabayko
changed the title
maxsharabayko
force-pushed
the
develop/busy_wait_build_opt
branch
2 times, most recently
from
092d42a to
4576a7a
Compare
|
My understanding is that in Visual Studio, inline asm is not supported for the x64 or ARM architectures [1], so I believe that srtcore/common.cpp would require a separate #elif for Windows simply using __nop. If anyone can verify that my understanding is correct, I'd be happy to make a PR |
|
@BananaHemic Yes, adding an intrinsinc nop for VS would make sense. I'm thinking whether it is better to create a separate PR, or add to this one. |
|
@maxlovic I'll make a separate PR once this one is merged |
maxsharabayko
force-pushed
the
develop/busy_wait_build_opt
branch
from
4576a7a to
8e5eb5c
Compare
|
Force-pushed a renaming of |
CMakeLists.txt
Outdated
| @@ -109,6 +109,8 @@ option(ENABLE_CXX_DEPS "Extra library dependencies in srt.pc for the CXX librari | |||
| option(USE_STATIC_LIBSTDCXX "Should use static rather than shared libstdc++" OFF) | |||
| option(ENABLE_INET_PTON "Set to OFF to prevent usage of inet_pton when building against modern SDKs while still requiring compatibility with older Windows versions, such as Windows XP, Windows Server 2003 etc." ON) | |||
| option(USE_OPENSSL_PC "Use pkg-config to find OpenSSL libraries" ON) | |||
| option(USE_BUSY_WAITING "Enable more accurate sending times at a cost of potentially higher CPU load" OFF) | |||
| option(ENABLE_UNEVEN_PACING "Enable and don't compensate uneven pacing of the sending (deviations of inter sending times)" OFF) | |||
There was a problem hiding this comment.
Isn't "odd" the opposite word for "even"?
Although I think the word that could better represent "deviations" would rather be "varying".
There was a problem hiding this comment.
ENABLE_IRREGULAR_PACINGENABLE_VARYING_PACINGENABLE_PACING_DEVIATION
I still like ENABLE_PACING_DEVIATION most.
There was a problem hiding this comment.
ENABLE_PACING_DIVERGENCE
There was a problem hiding this comment.
Well, "deviation" is still better than "divergence".
But inversion of the boolean meaning might help even more - options must begin with ENABLE, but can be set to true by default. So, to "enable this feature" you might want to say --disable-send-period-compensation (so the option would be ENABLE_SEND_PERIOD_COMPENSATION).
There was a problem hiding this comment.
Previously NO_BUSY_WAITING was defined in srt.h and udt.h, building SRT core without BUSY waiting and with "send period compensation".
I did this inversion to be preserve the default behavior without specifying those options. So if no options are provided, the core will be built without busy waiting and with send period compensation.
If SEND_PERIOD_COMPENSATION is introduced, it has to be ON by default. It is not a big deal, but still might result in unwanted behavior if someone builds SRT core as an external in 3rd party project e.g. without using our cmake. Not sure if someone does that though.
There was a problem hiding this comment.
@maxlovic,
it would be great and I'm sure it's possible to add busy_wait as well as ENABLE_UNEVEN_PACING as srt parameter(like latency, maxbw etc) for given connection and don't move them as compilation params.
When you use tools like ffmpeg or srt-trasmit you can have two versions without any problems but if you add SRT into the media server like we do you have a problem. Some people will definitely want to have some streams with default behavior and some streams with busy_wait.
Moreover once you introduce new build options you will have 2^n possible builds where n is a number of features and you will have to build srt with particular combination many times.
Options instead of compilation params is the only option if we want to have single SRT build with all possible use cases in it.
There was a problem hiding this comment.
@alexpokotilo Good point. Adding busy waiting as an option might introduce performance burden.
This PR is first of all to introduce a build option for users in 1.3.3.
Then PR #678 is scheduled to go in 1.3.4.
Then there are several ways to go. One way is that you suggested. One more is suggested in #673, where busy waiting is used for small wait intervals together with normal sleep.
I am also thinking about one timer shared between several sockets.
and ENABLE_UNEVEN_PACING
maxsharabayko
force-pushed
the
develop/busy_wait_build_opt
branch
from
8e5eb5c to
8db6a66
Compare
|
Finally, I decided to get rid of |
|
@BananaHemic PR is merged. You had a proposal for a __nop intrinsic for Windows. |
Haivision#711 Visual Studio doesn't support inline asm, but does allow a no-op intrinsic to be added
#711 Visual Studio doesn't support inline asm, but does allow a no-op intrinsic to be added
This PR extracts several things from #678 to be able to merge them without changing the behavior of SRT.
CTimer.DISABLED_SleeptoAccuracyto check the accuracy of the waiting function. Disabled by default, so will not be executed in CI.CTimer::sleepto()function to make its logic more obvious.Also refers to #673