Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release 1.2.0 #267

Merged
merged 12 commits into from
Nov 18, 2024
Merged

Release 1.2.0 #267

merged 12 commits into from
Nov 18, 2024

Conversation

sirknightj
Copy link
Contributor

@sirknightj sirknightj commented Nov 7, 2024

What was changed?

  • New thread API for custom stack size. New CMake flag to specify default stack size if no size is passed in. Changes are made to be backwards compatible.
  • Enhanced CI. Updated runner images, refactor for easier maintainability, speed it up.
    • New CMake flag combination action with report generation.
  • Code style report (apply linter) & auto-generation of patch files.
  • Fix a build issue (Correct missing defined in include file #265)

Why was it changed?

  • See the commits tab for more information on the individual pull requests.

How was it changed?

  • See the commits tab for more information on the individual pull requests.

What testing was done for the changes?

  • CI was revamped. The new CI is passing.
  • Canary for WebRTC-C updated to use this branch & no build issues or performance degradation.

Additional manual tests from producer-c using this dependency:

  1. Build producer-c with this branch, copied the test over and check that the new thread API is reachable.
./tst/producer_test --gtest_filter="InfoProviderApiTest.TestPicThread"
Note: Google Test filter = InfoProviderApiTest.TestPicThread
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from InfoProviderApiTest
[ RUN      ] InfoProviderApiTest.TestPicThread
2024-11-07 19:46:20.726 ERROR   defaultCreateThreadWithParams(): pthread_attr_setstacksize failed with 22
[       OK ] InfoProviderApiTest.TestPicThread (1 ms)
[----------] 1 test from InfoProviderApiTest (1 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (1 ms total)
[  PASSED  ] 1 test.
  1. Check that the build fails when both conflicting arguments are set:
$ cmake .. -DBUILD_DEPENDENCIES=OFF -DKVS_DEFAULT_STACK_SIZE=256 -DCONSTRAINED_DEVICE=ON
...
CMake Error at dependency/libkvspic/kvspic-src/CMakeLists.txt:62 (message):
  Conflicting parameters: KVS_DEFAULT_STACK_SIZE and CONSTRAINED_DEVICE
  cannot both be set.

-- Configuring incomplete, errors occurred!
  1. Pass in stack size only:
$ cmake .. -DBUILD_DEPENDENCIES=OFF -DKVS_DEFAULT_STACK_SIZE=65536
...
[100%] Completed 'libkvspic-download'
[100%] Built target libkvspic-download
-- Building with default stack size: 65536 bytes
...
  1. Added some local tests to Producer-C to check the stack size of threads with THREAD_CREATE and the new API as well. Tried it with a few different KVS_DEFAULT_STACK_SIZE's set and with CONSTRAINED_DEVICE and the new API. Each time, stack sizes changed depending on the parameters.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

sirknightj and others added 11 commits October 31, 2024 22:11
Fix unit test when log level env variable is not set
CI enhancements: Code linter, PR description, speed up
* Add API for specifying thread stack size (#243)

* Add API for specifying thread stack size

* Add compile time default stack size

* compile time definitions in cmake

* add local variable for stack size in API

* Adding temporary CMake Debug message

* Removed debug message in CMakeLists.txt, added CMAKE flag to readme

* Reset global variable before running new thread test

* Remove duplicate code and unused variables

* explicit cast

* missing )

* enforce pthread min stack size

* Change name of variable to have *_BYTES for readability

* Update variable name to include 'bytes' by request

* Addressing nit picks

* bound rand stack size value to not exceed max

* Remove rand() test on an OS wrapper API

* Wake up github

* Comment

* Remove lower bound checking for pthread

* Clang

* StackSize and ConstrainedDevice incompatible; Additional unit tests for the thread create API; Add new CI job and script to check CMake flag compatibility

* Add second case

* Fix typo

* Rename variables in the test

* Address comments and add log line in case of conflict again

---------

Co-authored-by: jdelapla <[email protected]>
@sirknightj sirknightj marked this pull request as draft November 7, 2024 22:05
src/utils/src/Thread.c Outdated Show resolved Hide resolved
* Change createThreadWithParams from size_t to versioned struct and add new negative test scenarios

* Remove designated initializers

* Fix capitalization

* Add message to clarify that stack size is not set

* Adjust status name and strengthen equality checks in the test

* Adjust name in the windows path as well

* Move the statuscode to the other location, adjust tooLargeThreadStack test for windows and linux

* Fix missing parenthesis

* Move the sleep into the guarded section

* Rename variable in the test
@sirknightj sirknightj marked this pull request as ready for review November 11, 2024 15:59
@sirknightj sirknightj merged commit c98c2a2 into master Nov 18, 2024
50 checks passed
@sirknightj sirknightj deleted the develop branch November 18, 2024 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants