search

FreeRDP / FreeRDP

FreeRDP is a free remote desktop protocol library and clients
http://www.freerdp.com/
Apache License 2.0
11.02k stars 14.86k forks source link

winpr: Performance overhead in ThreadPool task submission #10527

Closed chergert closed 2 months ago

chergert commented 2 months ago

Describe the bug

I'm doing some performance profiling of the FreeRDP support in GNOME 47 via gnome-remote-desktop using what will become Fedora 41. I notice that libfreerdp3.so is spending considerable time in winpr_SubmitThreadPoolWork() (on the machine hosting the session).

Having spent considerable time in my early career working on Mono and various runtime primitives, I can empathize with the amount of effort it takes to manage the scale of abstractions in winpr.

Specifically:

Expected behavior I'd expect the overhead of submitting threadpool work to be in the .01-.05% overhead range. Just roughly basing that on other threadpools I've written w/ semaphores to flag workers (which is what looks like is going on here) and profiling them for comparison.

Application details

Build configuration: BUILD_TESTING=OFF WINPR_HAVE_AIO_H=1 WINPR_HAVE_EXECINFO_BACKTRACE=1 WINPR_HAVE_EXECINFO_BACKTRACE_SYMBOLS=1 WINPR_HAVE_EXECINFO_BACKTRACE_SYMBOLS_FD=1 WINPR_HAVE_EXECINFO_HEADER=1 WINPR_HAVE_FCNTL_H=1 WINPR_HAVE_GETLOGIN_R=1 WINPR_HAVE_GETPWUID_R=1 WINPR_HAVE_INTTYPES_H=1 WINPR_HAVE_POLL_H=1 WINPR_HAVE_PTHREAD_MUTEX_TIMEDLOCK_LIB=1 WINPR_HAVE_PTHREAD_MUTEX_TIMEDLOCK_LIBS= WINPR_HAVE_PTHREAD_MUTEX_TIMEDLOCK_SYMBOL=1 WINPR_HAVE_STDBOOL_H=1 WINPR_HAVE_STDINT_H=1 WINPR_HAVE_STRNDUP=1 WINPR_HAVE_SYSLOG_H=1 WINPR_HAVE_SYS_EVENTFD_H=1 WINPR_HAVE_SYS_FILIO_H= WINPR_HAVE_SYS_SELECT_H=1 WINPR_HAVE_SYS_SOCKIO_H= WINPR_HAVE_SYS_TIMERFD_H=1 WINPR_HAVE_TM_GMTOFF=1 WINPR_HAVE_UNISTD_H=1 WINPR_HAVE_UNWIND_H=1 WITH_AAD=ON WITH_ABSOLUTE_PLUGIN_LOAD_PATHS=ON WITH_ADD_PLUGIN_TO_RPATH=OFF WITH_ALSA=ON WITH_BINARY_VERSIONING=OFF WITH_CAIRO=ON WITH_CCACHE=ON WITH_CHANNELS=ON WITH_CJSON_REQUIRED=OFF WITH_CLANG_FORMAT=ON WITH_CLIENT=ON WITH_CLIENT_AVAILABLE=1 WITH_CLIENT_CHANNELS=ON WITH_CLIENT_CHANNELS_AVAILABLE=1 WITH_CLIENT_COMMON=ON WITH_CLIENT_INTERFACE=OFF WITH_CLIENT_SDL=ON WITH_CLIENT_SDL2=ON WITH_CLIENT_SDL3=OFF WITH_CLIENT_SDL_AVAILABLE=1 WITH_CLIENT_SDL_VERSIONED=OFF WITH_CUPS=ON WITH_DEBUG_ALL=OFF WITH_DEBUG_CAPABILITIES=OFF WITH_DEBUG_CERTIFICATE=OFF WITH_DEBUG_CHANNELS=OFF WITH_DEBUG_CLIPRDR=OFF WITH_DEBUG_CODECS=OFF WITH_DEBUG_DVC=OFF WITH_DEBUG_EVENTS=OFF WITH_DEBUG_KBD=OFF WITH_DEBUG_LICENSE=OFF WITH_DEBUG_MUTEX=OFF WITH_DEBUG_NEGO=OFF WITH_DEBUG_NLA=OFF WITH_DEBUG_NTLM=OFF WITH_DEBUG_RAIL=OFF WITH_DEBUG_RDP=OFF WITH_DEBUG_RDPDR=OFF WITH_DEBUG_RDPEI=OFF WITH_DEBUG_RDPGFX=OFF WITH_DEBUG_REDIR=OFF WITH_DEBUG_RFX=OFF WITH_DEBUG_RINGBUFFER=OFF WITH_DEBUG_SCARD=OFF WITH_DEBUG_SCHANNEL=OFF WITH_DEBUG_SDL_EVENTS=OFF WITH_DEBUG_SDL_KBD_EVENTS=OFF WITH_DEBUG_SND=OFF WITH_DEBUG_SVC=OFF WITH_DEBUG_SYMBOLS=OFF WITH_DEBUG_THREADS=OFF WITH_DEBUG_TIMEZONE=OFF WITH_DEBUG_TRANSPORT=OFF WITH_DEBUG_TSG=OFF WITH_DEBUG_TSMF=OFF WITH_DEBUG_TSMF_AVAILABLE=0 WITH_DEBUG_URBDRC=OFF WITH_DEBUG_WND=OFF WITH_DEBUG_X11=OFF WITH_DEBUG_X11_LOCAL_MOVESIZE=OFF WITH_DEBUG_XV=OFF WITH_DIRECTFB=OFF WITH_DSP_EXPERIMENTAL=OFF WITH_DSP_FFMPEG=OFF WITH_DSP_FFMPEG=OFF WITH_DSP_FFMPEG_AVAILABLE=0 WITH_EVENTFD_READ_WRITE=1 WITH_FAAC=OFF WITH_FAAD2=OFF WITH_FDK_AAC=ON WITH_FFMPEG=OFF WITH_FREERDP_DEPRECATED=OFF WITH_FREERDP_DEPRECATED_COMMANDLINE=OFF WITH_FULL_CONFIG_PATH=OFF WITH_FUSE=ON WITH_GFX_FRAME_DUMP=OFF WITH_GFX_H264=ON WITH_GPROF=OFF WITH_GSM=ON WITH_ICU=ON WITH_INTERNAL_MD4=OFF WITH_INTERNAL_MD5=OFF WITH_INTERNAL_RC4=OFF WITH_IPP=OFF WITH_JPEG=ON WITH_JSONC_REQUIRED=ON WITH_JSON_DISABLED=OFF WITH_KEYBOARD_LAYOUT_FROM_FILE=ON WITH_KRB5=ON WITH_KRB5_NO_NTLM_FALLBACK=OFF WITH_LAME=ON WITH_LIBRARY_VERSIONING=ON WITH_LIBRESSL=OFF WITH_LODEPNG=OFF WITH_MACAUDIO=OFF WITH_MACAUDIO_AVAILABLE=0 WITH_MANPAGES=ON WITH_MBEDTLS=OFF WITH_NATIVE_SSPI=OFF WITH_NEON=OFF WITH_OPENCL=OFF WITH_OPENH264=TRUE WITH_OPENH264=TRUE WITH_OPENH264_LOADING=OFF WITH_OPENSSL=ON WITH_OPUS=ON WITH_OSS=ON WITH_PCSC=ON WITH_PKCS11=ON WITH_PLATFORM_SERVER=ON WITH_POLL=ON WITH_PROFILER=OFF WITH_PROXY=ON WITH_PROXY_APP=ON WITH_PROXY_EMULATE_SMARTCARD=OFF WITH_PROXY_MODULES=ON WITH_PULSE=ON WITH_RDTK=ON WITH_SAMPLE=OFF WITH_SANITIZE_ADDRESS=OFF WITH_SANITIZE_ADDRESS_AVAILABLE=1 WITH_SANITIZE_MEMORY=OFF WITH_SANITIZE_MEMORY_AVAILABLE=1 WITH_SANITIZE_THREAD=OFF WITH_SANITIZE_THREAD_AVAILABLE=1 WITH_SDL_IMAGE_DIALOGS=OFF WITH_SDL_LINK_SHARED=ON WITH_SERVER=ON WITH_SERVER_CHANNELS=ON WITH_SERVER_INTERFACE=ON WITH_SHADOW=ON WITH_SHADOW_MAC=ON WITH_SHADOW_X11=ON WITH_SMARTCARD_EMULATE=ON WITH_SMARTCARD_INSPECT=OFF WITH_SMARTCARD_PCSC=ON WITH_SOXR=ON WITH_SSE2=ON WITH_SWSCALE=OFF WITH_SYSTEMD=ON WITH_THIRD_PARTY=OFF WITH_TIMEZONE_COMPILED=OFF WITH_TIMEZONE_FROM_FILE=ON WITH_TIMEZONE_ICU=OFF WITH_UNICODE_BUILTIN=OFF WITH_URIPARSER=ON WITH_VAAPI=OFF WITH_VAAPI=OFF WITH_VAAPI_AVAILABLE=0 WITH_VALGRIND_MEMCHECK=OFF WITH_VALGRIND_MEMCHECK_AVAILABLE=1 WITH_VERBOSE_WINPR_ASSERT=ON WITH_VIDEO_FFMPEG=OFF WITH_VIDEO_FFMPEG=OFF WITH_VIDEO_FFMPEG_AVAILABLE=0 WITH_WAYLAND=ON WITH_WEBVIEW=OFF WITH_WINPR_DEPRECATED=OFF WITH_WINPR_JSON=ON WITH_WINPR_TOOLS=ON WITH_WIN_CONSOLE=ON WITH_X11=ON WITH_XCURSOR=ON WITH_XEXT=ON WITH_XFIXES=ON WITH_XI=ON WITH_XINERAMA=ON WITH_XKBFILE=ON WITH_XRANDR=ON WITH_XRENDER=ON WITH_XTEST=ON WITH_XV=ON WITH_ZLIB=ON
Build type:          
CFLAGS:              -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wno-error=incompatible-pointer-types -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64 -march=x86-64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -mtls-dialect=gnu2 -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -Wall -Wpedantic -Wno-padded -Wno-cast-align -Wno-declaration-after-statement -fPIC -Wall -fvisibility=hidden -Wimplicit-function-declaration -Wredundant-decls -g -fno-omit-frame-pointer
Compiler:            GNU, 14.1.1
Target architecture: x64
Keyboard Shortcuts:
    <Right CTRL>
        releases keyboard and mouse grab
    <CTRL>+<ALT>+<Return>
        toggles fullscreen state of the application
    <CTRL>+<ALT>+c
        toggles remote control in a remote assistance session
    Action Script
        Executes a predefined script on key press.
        Should the script not exist it is ignored.
        Scripts can be provided at the default location ~/.config/freerdp/action.sh or as command line argument /action:script:<path>
        The script will receive the current key combination as argument.
        The output of the script is parsed for 'key-local' which tells that the script used the key combination, otherwise the combination is forwarded to the remote.
chergert commented 2 months ago

Looking at CountdownEvent, and assuming it's a fairly straightforward implementation of the C# equivalent, I see it doing ResetEvent() in the AddCount() case.

Shouldn't it never need to reset when adding, only potentially when signalling?

akallabeth commented 2 months ago

@chergert thank you for having a look at that. not sure why it was implemented that way (older than my affiliation with the project) but the condition could at least be improved to only reset when countdonw->count == 0 before adding.