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

new(ci): enable kernel testing on PRs. #1935

Merged
merged 2 commits into from
Jun 26, 2024
Merged

Conversation

FedeDP
Copy link
Contributor

@FedeDP FedeDP commented Jun 26, 2024

What type of PR is this?

/kind feature

Any specific area of the project related to this PR?

/area CI

Does this PR require a change in the driver versions?

What this PR does / why we need it:

This PR enables kernel-testing framework on PRs.
I discovered that, for self hosted runners, there is no parallelism: each job's request is enqueued.
The only parallelism would happen if we had multiple self hosted runners under same tags, then the jobs would be distributed among them.
See https://github.com/orgs/community/discussions/26769#discussioncomment-3253321

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

Signed-off-by: Federico Di Pierro <[email protected]>
@FedeDP
Copy link
Contributor Author

FedeDP commented Jun 26, 2024

/milestone 0.18.0

NOTE: i want to also add a comment on the PR with the results :)

@poiana poiana requested review from hbrueckner and Molter73 June 26, 2024 07:52
@poiana poiana added this to the 0.18.0 milestone Jun 26, 2024
Copy link

Perf diff from master - unit tests

    10.26%     +1.11%  [.] sinsp_parser::reset
     3.47%     +0.99%  [.] sinsp_evt::load_params
     2.42%     -0.74%  [.] scap_event_decode_params
     2.21%     -0.61%  [.] std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char const*>
     1.26%     -0.59%  [.] sinsp_evt::get_ts
     4.57%     +0.49%  [.] sinsp_evt::get_type
     0.92%     +0.47%  [.] 0x00000000000e7390
     1.47%     -0.35%  [.] std::_Hashtable<long, std::pair<long const, std::shared_ptr<sinsp_threadinfo> >, std::allocator<std::pair<long const, std::shared_ptr<sinsp_threadinfo> > >, std::__detail::_Select1st, std::equal_to<long>, std::hash<long>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::_M_find_before_node
     4.94%     -0.33%  [.] next
     0.47%     +0.33%  [.] libsinsp::state::stl_container_table_adapter<std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, libsinsp::state::value_table_entry_adapter<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, libsinsp::state::value_table_entry_adapter<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >::dynamic_fields_t>::stl_container_table_adapter

Perf diff from master - scap file

    24.30%     -7.77%  [.] sinsp_filter_check::extract_nocache
    12.02%     -4.67%  [.] sinsp_filter_check::tostring
     2.36%     +3.38%  [.] sinsp_thread_manager::find_thread
     1.02%     +2.48%  [.] gzfile_read
     2.38%     +1.82%  [.] sinsp_thread_manager::get_thread_ref
     4.87%     -1.77%  [.] libsinsp::runc::match_one_container_id
     2.44%     +1.77%  [.] sinsp_filter_check_thread::extract_single
     4.87%     -1.53%  [.] sinsp_parser::process_event
     4.86%     -1.52%  [.] sinsp_parser::reset
     4.54%     -1.52%  [.] libsinsp::runc::match_container_id

Heap diff from master - unit tests

total runtime: -0.10s.
calls to allocation functions: -157 (1554/s)
temporary memory allocations: 71 (-702/s)
peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

Heap diff from master - scap file

total runtime: 0.01s.
calls to allocation functions: 0 (0/s)
temporary memory allocations: -2 (-222/s)
peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

@poiana poiana added size/M and removed size/S labels Jun 26, 2024
@FedeDP FedeDP force-pushed the new/enable_kernel_testing_PR branch from 2713641 to c179fe3 Compare June 26, 2024 08:23
@poiana poiana added size/L and removed size/M labels Jun 26, 2024
Copy link
Contributor Author

@FedeDP FedeDP Jun 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as create-comment.yml for perf but with diffeernt workflow_run trigger.

@@ -185,7 +185,7 @@ jobs:
run: echo "GIT_BRANCH=${GITHUB_HEAD_REF:-${GITHUB_REF#refs/heads/}}" >> $GITHUB_ENV

- name: Build and test drivers on ppc64le node via ssh
if: needs.paths-filter.outputs.driver_needs_rebuild
if: needs.paths-filter.outputs.driver == 'true' || needs.paths-filter.outputs.libscap == 'true' || needs.paths-filter.outputs.libpman == 'true'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a fix :)

libsversion: ${{ github.event.pull_request.head.sha }}
secrets: inherit

kernel-tests-upload:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once kernel-tests-dev finish, a new job will take care of uploading PR comment and info to be used by the matching create-comment.
Note: reusable_workflows do not support running as step, only job-wide ;)

echo ${{ github.event.number }} > ./pr/NR
touch ./pr/COMMENT
echo "# X64 kernel testing matrix" >> ./pr/COMMENT
echo "$(head -n $(grep -n -v -m1 '^|' matrix_X64/matrix.md | awk -F':' '{ print $1 }') matrix_X64/matrix.md)" >> ./pr/COMMENT
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Magic to get the first line that does not start with |, and head until it, so that we only print matrixes and not errors (full matrixes are still available as artifacts though).

@FedeDP FedeDP changed the title wip: new(ci): enable kernel testing on PRs. new(ci): enable kernel testing on PRs. Jun 26, 2024
Copy link

Perf diff from master - unit tests

     3.45%     +1.31%  [.] sinsp_evt::load_params
     4.92%     +1.21%  [.] next
     6.33%     -1.16%  [.] sinsp::next
     2.20%     -0.93%  [.] std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char const*>
     2.41%     -0.79%  [.] scap_event_decode_params
     4.57%     +0.79%  [.] sinsp_parser::process_event
     3.72%     -0.75%  [.] sinsp_thread_manager::get_thread_ref
     4.55%     +0.72%  [.] sinsp_evt::get_type
     2.15%     -0.59%  [.] sinsp::fetch_next_event
     1.36%     -0.52%  [.] libsinsp::events::is_unknown_event

Perf diff from master - scap file

    29.13%     -8.83%  [.] sinsp_filter_check::extract_nocache
    14.41%     -4.24%  [.] sinsp_filter_check::tostring
     7.02%     +3.00%  [.] sinsp_threadinfo::~sinsp_threadinfo
    11.55%     -2.63%  [.] sinsp_evt_formatter::tostring_withformat
     5.44%     +2.63%  [.] std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char const*>
     2.91%     +2.00%  [.] std::_Hashtable<long, std::pair<long const, std::shared_ptr<sinsp_threadinfo> >, std::allocator<std::pair<long const, std::shared_ptr<sinsp_threadinfo> > >, std::__detail::_Select1st, std::equal_to<long>, std::hash<long>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::_M_find_before_node
     1.23%     +1.21%  [.] gzfile_read
     5.84%     +0.56%  [.] sinsp_parser::process_event
     5.81%     +0.53%  [.] next
     5.44%     +0.47%  [.] libsinsp::runc::match_container_id

Heap diff from master - unit tests

total runtime: -0.06s.
calls to allocation functions: -583 (10600/s)
temporary memory allocations: -113 (2054/s)
peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

Heap diff from master - scap file

total runtime: 0.00s.
calls to allocation functions: 0 (0/s)
temporary memory allocations: -4 (-4000/s)
peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

Copy link
Member

@Andreagit97 Andreagit97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

Copy link
Contributor

@incertum incertum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

@poiana
Copy link
Contributor

poiana commented Jun 26, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Andreagit97, FedeDP, incertum

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [Andreagit97,FedeDP,incertum]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@poiana poiana merged commit fbf88d6 into master Jun 26, 2024
43 checks passed
@poiana poiana deleted the new/enable_kernel_testing_PR branch June 26, 2024 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants