maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Holyfoot!

On Jul 01, holyfoot@xxxxxxxxxxxx wrote:
> revno: 4270
> revision-id: holyfoot@xxxxxxxxxxxx-20140701104158-4gj5ztnjqlr1alwa
> parent: holyfoot@xxxxxxxxxxxx-20140630193024-56wpcvwwy24kuzt4
> committer: Alexey Botchkov <holyfoot@xxxxxxxxxxxx>
> branch nick: mdev-5756
> timestamp: Tue 2014-07-01 15:41:58 +0500
> message:
>   MDEV-5756 CMake option to build without thread pool.
>           Check if the threadpool is available on the system and set HAVE_POOL_OF_THREADS respectively.

> === modified file 'sql/CMakeLists.txt'
> --- a/sql/CMakeLists.txt	2014-05-30 11:24:25 +0000
> +++ b/sql/CMakeLists.txt	2014-07-01 10:41:58 +0000
> @@ -30,7 +30,15 @@ ${CMAKE_CURRENT_BINARY_DIR}/lex_hash.h 
>  
>  SET_SOURCE_FILES_PROPERTIES(${GEN_SOURCES} PROPERTIES GENERATED 1)
>  
> -ADD_DEFINITIONS(-DMYSQL_SERVER -DHAVE_EVENT_SCHEDULER -DHAVE_POOL_OF_THREADS) 
> +ADD_DEFINITIONS(-DMYSQL_SERVER -DHAVE_EVENT_SCHEDULER) 
> +
> +IF (CMAKE_SYSTEM_NAME MATCHES "Linux" OR
> +    CMAKE_SYSTEM_NAME MATCHES "Windows" OR
> +    CMAKE_SYSTEM_NAME MATCHES "SunOS" OR
> +    HAVE_KQUEUE)
> + ADD_DEFINITIONS(-DHAVE_POOL_OF_THREADS) 
> +ENDIF()
> +
>  IF(SSL_DEFINES)
>   ADD_DEFINITIONS(${SSL_DEFINES})
>  ENDIF()

Did you test it on any of the platforms that that didn't support
thread pool? If you would've done it, you'd noticed that your patch
doesn't work. Because scheduler.h contains:

#undef HAVE_POOL_OF_THREADS
#if !defined(EMBEDDED_LIBRARY) && !defined(_AIX)
#define HAVE_POOL_OF_THREADS 1

So, HAVE_POOL_OF_THREADS is only disabled in embedded and on AIX.
The AIX part was added recently by Bar, as a quick fix to make MariaDB
to compile on AIX. Your patch should remove the AIX test from
scheduler.h. Actually, you should remove all #define and #undef
HAVE_POOL_OF_THREADS from all files, only CMakeLists.txt should do it.

Otherwise it's ok. I don't like that it explicitly checks for OS (Linux,
Windows, SunOS) instead of checking for specific functions, libraries,
or headers. But it's the same check that threadpool_unix.cc does,
so it's ok.

Regards,
Sergei