switch-off uthread on not supported platform #66
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Rain Mark <rain.by.zhou@gmail.com>
RainMark
requested review from
ChuanqiXu9 and
qicosmos
and removed request for
ChuanqiXu9
|
Have you tested it on Mac? |
ChuanqiXu9
reviewed
Mar 23, 2022
| add_library(async_simple SHARED ${SRCS}) | ||
| target_link_libraries(async_simple ${deplibs}) | ||
| target_link_libraries(async_simple_static ${deplibs}) | ||
| if(SRCS) |
There was a problem hiding this comment.
Is this condition necessary?
There was a problem hiding this comment.
the SRCS is null after disabled uthread
cmake will throw error if SRCS is empty
There was a problem hiding this comment.
How about adding an empty src file to reduce the branches? I feel things is more uniform in this way.
There was a problem hiding this comment.
i prefer to move some not effect performance logic to cpp source, empty src file so ugly
There was a problem hiding this comment.
Event.h not template class, we can move some implement to cpp
There was a problem hiding this comment.
CountEvent contains some atomic operations. I feel it might affect the performance. Oh, if you feel empty src file is ugly, I think we could reduce the branch later once we have actual src files.
There was a problem hiding this comment.
CountEvent contains some atomic operations. I feel it might affect the performance. Oh, if you feel empty src file is ugly, I think we could reduce the branch later once we have actual src files.
OK，I will add a comment to make others note it
| @@ -1,7 +1,7 @@ | |||
| file(GLOB coro_test_src "*.cpp") | |||
| add_executable(async_simple_coro_test ${coro_test_src} ${PROJECT_SOURCE_DIR}/async_simple/test/dotest.cpp) | |||
|
|
|||
| target_link_libraries(async_simple_coro_test async_simple ${deplibs} ${testdeplibs}) | |||
| target_link_libraries(async_simple_coro_test ${deplibs} ${testdeplibs}) | |||
There was a problem hiding this comment.
Would it be harmful to remain it? If no, I prefer to remain it.
There was a problem hiding this comment.
if we keep it, we must move some source code to cpp file, feels not suitable
|
I tested on Mac with clang compiler, there are some compile errors:
|
yes, i passed all test on libc++ mode, but got one error about link -lstdc++fs, we need fix it in another pr $ make -j4
[ 5%] Building CXX object async_simple/test/CMakeFiles/async_simple_test.dir/FutureStateTest.cpp.o
[ 5%] Building CXX object async_simple/coro/test/CMakeFiles/async_simple_coro_test.dir/FutureAwaiterTest.cpp.o
[ 7%] Building CXX object async_simple/executors/test/CMakeFiles/async_simple_executor_test.dir/SimpleIOExecutorTest.cpp.o
[ 10%] Building CXX object async_simple/util/test/CMakeFiles/async_simple_util_test.dir/ThreadPoolTest.cpp.o
[ 13%] Building CXX object async_simple/executors/test/CMakeFiles/async_simple_executor_test.dir/__/__/test/dotest.cpp.o
[ 15%] Linking CXX executable async_simple_executor_test
[ 15%] Built target async_simple_executor_test
[ 18%] Building CXX object async_simple/coro/test/CMakeFiles/async_simple_coro_test.dir/LazyTest.cpp.o
[ 21%] Building CXX object async_simple/util/test/CMakeFiles/async_simple_util_test.dir/__/__/test/dotest.cpp.o
[ 23%] Building CXX object async_simple/coro/test/CMakeFiles/async_simple_coro_test.dir/SleepTest.cpp.o
[ 26%] Building CXX object async_simple/test/CMakeFiles/async_simple_test.dir/FutureTest.cpp.o
[ 28%] Linking CXX executable async_simple_util_test
[ 28%] Built target async_simple_util_test
[ 31%] Building CXX object async_simple/coro/test/CMakeFiles/async_simple_coro_test.dir/TraitsTest.cpp.o
[ 34%] Building CXX object demo_example/CMakeFiles/smtp_client.dir/smtp/smtp_client.cpp.o
[ 36%] Building CXX object demo_example/CMakeFiles/block_http_server.dir/http/block_http/block_http_server.cpp.o
[ 39%] Linking CXX executable block_http_server
[ 39%] Built target block_http_server
[ 42%] Building CXX object async_simple/coro/test/CMakeFiles/async_simple_coro_test.dir/ViaCoroutineTest.cpp.o
[ 44%] Linking CXX executable smtp_client
ld: library not found for -lstdc++fs
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [demo_example/smtp_client] Error 1
make[1]: *** [demo_example/CMakeFiles/smtp_client.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
[ 47%] Building CXX object async_simple/test/CMakeFiles/async_simple_test.dir/TryTest.cpp.o
[ 50%] Building CXX object async_simple/coro/test/CMakeFiles/async_simple_coro_test.dir/__/__/test/dotest.cpp.o
[ 52%] Building CXX object async_simple/test/CMakeFiles/async_simple_test.dir/dotest.cpp.o
[ 55%] Linking CXX executable async_simple_test
[ 55%] Built target async_simple_test
[ 57%] Linking CXX executable async_simple_coro_test
[ 57%] Built target async_simple_coro_test
make: *** [all] Error 2-lstdc++fs removed $ make -j4
-- CMAKE_MODULE_PATH: /Users/rain/Project/async_simple/cmake
-- FindAio.cmake:
CMake Warning (dev) at /usr/local/Cellar/cmake/3.20.3/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:438 (message):
The package name passed to `find_package_handle_standard_args` (aio) does
not match the name of the calling package (Aio). This can lead to problems
in calling code that expects `find_package` result variables (e.g.,
`_FOUND`) to follow a certain pattern.
Call Stack (most recent call first):
cmake/FindAio.cmake:22 (find_package_handle_standard_args)
CMakeLists.txt:8 (find_package)
This warning is for project developers. Use -Wno-dev to suppress it.
-- Could NOT find aio (missing: LIBAIO_LIBRARIES)
CMake Warning (dev) at /usr/local/Cellar/cmake/3.20.3/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:438 (message):
The package name passed to `find_package_handle_standard_args` (gmock) does
not match the name of the calling package (GMock). This can lead to
problems in calling code that expects `find_package` result variables
(e.g., `_FOUND`) to follow a certain pattern.
Call Stack (most recent call first):
cmake/FindGMock.cmake:25 (find_package_handle_standard_args)
CMakeLists.txt:9 (find_package)
This warning is for project developers. Use -Wno-dev to suppress it.
CMake Warning (dev) at /usr/local/Cellar/cmake/3.20.3/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:438 (message):
The package name passed to `find_package_handle_standard_args` (gtest) does
not match the name of the calling package (GTest). This can lead to
problems in calling code that expects `find_package` result variables
(e.g., `_FOUND`) to follow a certain pattern.
Call Stack (most recent call first):
cmake/FindGTest.cmake:25 (find_package_handle_standard_args)
CMakeLists.txt:10 (find_package)
This warning is for project developers. Use -Wno-dev to suppress it.
CMake Warning (dev) at /usr/local/Cellar/cmake/3.20.3/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:438 (message):
The package name passed to `find_package_handle_standard_args` (benchmark)
does not match the name of the calling package (Benchmark). This can lead
to problems in calling code that expects `find_package` result variables
(e.g., `_FOUND`) to follow a certain pattern.
Call Stack (most recent call first):
cmake/FindBenchmark.cmake:25 (find_package_handle_standard_args)
CMakeLists.txt:11 (find_package)
This warning is for project developers. Use -Wno-dev to suppress it.
-- Could NOT find benchmark (missing: BENCHMARK_LIBRARIES BENCHMARK_INCLUDE_DIR)
-- GTest found.
-- GTest: /usr/local/include, /usr/local/lib/libgtest.a
-- GMock found.
-- GMock: /usr/local/include, /usr/local/lib/libgmock.a
-- Use flag -m64
-- Skipping benchmarks
-- Configuring done
-- Generating done
-- Build files have been written to: /Users/rain/Project/async_simple/oxs
Consolidate compiler generated dependencies of target async_simple_test
Consolidate compiler generated dependencies of target async_simple_util_test
Consolidate compiler generated dependencies of target async_simple_coro_test
Consolidate compiler generated dependencies of target async_simple_executor_test
[ 7%] Built target async_simple_executor_test
[ 15%] Built target async_simple_util_test
[ 28%] Built target async_simple_test
Consolidate compiler generated dependencies of target smtp_client
[ 47%] Built target async_simple_coro_test
Consolidate compiler generated dependencies of target block_http_server
[ 50%] Building CXX object demo_example/CMakeFiles/http_client.dir/http/coroutine_http/http_client.cpp.o
[ 52%] Building CXX object demo_example/CMakeFiles/CountChar.dir/CountChar.cpp.o
[ 55%] Linking CXX executable smtp_client
[ 60%] Built target block_http_server
[ 63%] Building CXX object demo_example/CMakeFiles/http_server.dir/http/coroutine_http/http_server.cpp.o
[ 65%] Built target smtp_client
[ 68%] Building CXX object demo_example/CMakeFiles/block_echo_client.dir/block_echo_client.cpp.o
[ 71%] Linking CXX executable CountChar
[ 71%] Built target CountChar
[ 73%] Building CXX object demo_example/CMakeFiles/block_echo_server.dir/block_echo_server.cpp.o
[ 76%] Linking CXX executable block_echo_client
[ 76%] Built target block_echo_client
[ 78%] Building CXX object demo_example/CMakeFiles/async_echo_server.dir/async_echo_server.cpp.o
[ 81%] Linking CXX executable http_client
[ 81%] Built target http_client
[ 84%] Building CXX object demo_example/CMakeFiles/async_echo_client.dir/async_echo_client.cpp.o
[ 86%] Linking CXX executable block_echo_server
[ 86%] Built target block_echo_server
[ 89%] Building CXX object demo_example/CMakeFiles/ReadFiles.dir/ReadFiles.cpp.o
[ 92%] Linking CXX executable http_server
[ 92%] Built target http_server
[ 94%] Linking CXX executable async_echo_server
[ 94%] Built target async_echo_server
[ 97%] Linking CXX executable async_echo_client
[ 97%] Built target async_echo_client
[100%] Linking CXX executable ReadFiles
[100%] Built target ReadFiles
12:08:48 rain@RaindeMacBook-Pro ...Project/async_simple/oxs off_uthread ✘ ✹ ✭ 23s
$ make test
Running tests...
Test project /Users/rain/Project/async_simple/oxs
Start 1: run_async_simple_test
1/4 Test #1: run_async_simple_test ............ Passed 1.49 sec
Start 2: run_async_simple_util_test
2/4 Test #2: run_async_simple_util_test ....... Passed 0.29 sec
Start 3: run_async_simple_coro_test
3/4 Test #3: run_async_simple_coro_test ....... Passed 10.93 sec
Start 4: run_async_simple_executor_test
4/4 Test #4: run_async_simple_executor_test ... Passed 0.28 sec
100% tests passed, 0 tests failed out of 4
Total Test time (real) = 13.00 sec
macOS 12.1 (21C52) |
It looks like the version of your compiler is not high enough to support concept. |
|
Good, could you also update the doc: how to compile the library on Mac? |
ok |
|
stdc++fs related issue: https://gitlab.kitware.com/cmake/cmake/-/issues/17834 I tested result
|
ChuanqiXu9
approved these changes
Mar 23, 2022
|
Should revert because of #69 |
qicosmos
added a commit
that referenced
this pull request
Mar 23, 2022
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Rain Mark rain.by.zhou@gmail.com
Why
Close #39