chore(libsinsp): avoid useless allocation #2148

therealbobo · 2024-11-05T10:07:46Z

What type of PR is this?

Uncomment one (or more) /kind <> lines:

/kind bug

/kind cleanup

/kind design

/kind documentation

/kind failing-test

/kind feature

Any specific area of the project related to this PR?

Uncomment one (or more) /area <> lines:

/area API-version

/area build

/area CI

/area driver-kmod

/area driver-bpf

/area driver-modern-bpf

/area libscap-engine-bpf

/area libscap-engine-gvisor

/area libscap-engine-kmod

/area libscap-engine-modern-bpf

/area libscap-engine-nodriver

/area libscap-engine-noop

/area libscap-engine-source-plugin

/area libscap-engine-savefile

/area libscap

/area libpman

/area libsinsp

/area tests

/area proposals

Does this PR require a change in the driver versions?

/version driver-API-version-major

/version driver-API-version-minor

/version driver-API-version-patch

/version driver-SCHEMA-version-major

/version driver-SCHEMA-version-minor

/version driver-SCHEMA-version-patch

What this PR does / why we need it:

Which issue(s) this PR fixes:

This avoids the allocation of a temporary object: to do that emplace_back needs the arguments of the constructor underlying object.

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

Signed-off-by: Roberto Scolaro <[email protected]>

github-actions · 2024-11-05T10:13:45Z

Perf diff from master - unit tests

     5.01%     -1.24%  [.] sinsp_parser::process_event
     3.99%     -1.19%  [.] gzfile_read
     2.44%     +0.95%  [.] sinsp_thread_manager::find_thread
     6.49%     -0.90%  [.] sinsp::next
     0.75%     +0.76%  [.] sinsp_parser::event_cleanup
     0.70%     +0.75%  [.] sinsp_filter_check::parse_field_name
     1.81%     -0.72%  [.] 0x00000000000ea3b0
     1.08%     +0.56%  [.] libsinsp::sinsp_suppress::process_event
     0.70%     +0.48%  [.] std::vector<sinsp_evt_param, std::allocator<sinsp_evt_param> >::emplace_back<sinsp_evt*, unsigned int&, char const*, unsigned long&>
     0.21%     +0.45%  [.] sinsp_parser::parse_clone_exit_child

Heap diff from master - unit tests

peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

Heap diff from master - scap file

peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

Benchmarks diff from master

Comparing gbench_data.json to /root/actions-runner/_work/libs/libs/build/gbench_data.json
Benchmark                                                         Time             CPU      Time Old      Time New       CPU Old       CPU New
----------------------------------------------------------------------------------------------------------------------------------------------
BM_sinsp_split_mean                                            -0.0701         -0.0701           152           141           152           141
BM_sinsp_split_median                                          -0.0753         -0.0753           153           141           153           141
BM_sinsp_split_stddev                                          -0.2438         -0.2436             1             1             1             1
BM_sinsp_split_cv                                              -0.1868         -0.1866             0             0             0             0
BM_sinsp_concatenate_paths_relative_path_mean                  -0.0915         -0.0915            62            56            62            56
BM_sinsp_concatenate_paths_relative_path_median                -0.1041         -0.1041            62            56            62            56
BM_sinsp_concatenate_paths_relative_path_stddev                +1.1083         +1.1074             1             1             1             1
BM_sinsp_concatenate_paths_relative_path_cv                    +1.3206         +1.3197             0             0             0             0
BM_sinsp_concatenate_paths_empty_path_mean                     -0.0009         -0.0009            24            24            24            24
BM_sinsp_concatenate_paths_empty_path_median                   -0.0019         -0.0019            24            24            24            24
BM_sinsp_concatenate_paths_empty_path_stddev                   +0.0044         +0.0084             0             0             0             0
BM_sinsp_concatenate_paths_empty_path_cv                       +0.0053         +0.0093             0             0             0             0
BM_sinsp_concatenate_paths_absolute_path_mean                  -0.1051         -0.1051            63            56            62            56
BM_sinsp_concatenate_paths_absolute_path_median                -0.1146         -0.1146            63            56            63            56
BM_sinsp_concatenate_paths_absolute_path_stddev                -0.7316         -0.7316             1             0             1             0
BM_sinsp_concatenate_paths_absolute_path_cv                    -0.7001         -0.7001             0             0             0             0
BM_sinsp_split_container_image_mean                            -0.0261         -0.0260           398           387           398           387
BM_sinsp_split_container_image_median                          -0.0259         -0.0259           397           387           397           387
BM_sinsp_split_container_image_stddev                          +1.0519         +1.0504             1             3             1             3
BM_sinsp_split_container_image_cv                              +1.1068         +1.1053             0             0             0             0

codecov · 2024-11-05T10:18:35Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.67%. Comparing base (f82c686) to head (50ffd82).
Report is 8 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2148      +/-   ##
==========================================
- Coverage   74.69%   74.67%   -0.02%     
==========================================
  Files         254      254              
  Lines       33502    33513      +11     
  Branches     5746     5727      -19     
==========================================
+ Hits        25025    25027       +2     
+ Misses       8477     8469       -8     
- Partials        0       17      +17

Flag	Coverage Δ
libsinsp	`74.67% <100.00%> (-0.02%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

FedeDP · 2024-11-05T10:46:18Z

BM_sinsp_split_mean -0.0706 -0.0706 151 140 151 140
BM_sinsp_split_median -0.0709 -0.0709 151 140 151 140

Confirmed by benchmarks :)

FedeDP · 2024-11-05T10:46:23Z

/milestone 0.19.0

FedeDP

/approve

poiana · 2024-11-05T10:46:42Z

LGTM label has been added.

Git tree hash: 8b980582f5fbd3095d457c3066c8338503043b3d

Andreagit97

/approve

Signed-off-by: Roberto Scolaro <[email protected]>

FedeDP · 2024-11-07T15:33:28Z

userspace/libsinsp/test/async_key_value_source.ut.cpp

@@ -358,15 +358,15 @@ TEST(async_key_value_source_test, prune_old_metadata) {
 	ASSERT_FALSE(source.lookup(key1, response));

 	// Wait long enough for the old entry to require pruning
-	std::this_thread::sleep_for(std::chrono::milliseconds(2 * TTL_MS));
+	usleep(1000 * 2 * TTL_MS);


I can't believe this is a sensible fix 🤣 any idea why?
Anyway, just re-triggered the CI; i will trigger it many times before proceeding.

I was really out of ideas 😓 and I'm surprise as much as you. The problem should be related to the lifetime of the object and that was the only function that was related 😅

jasondellaluce

lol

poiana · 2024-11-07T17:24:38Z

LGTM label has been added.

Git tree hash: 9eed0aeb84a893aa11e528a53b1453002747c26f

FedeDP

/approve

poiana · 2024-11-07T17:25:35Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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]

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

chore(libsinsp): avoid useless allocation

d5a2786

Signed-off-by: Roberto Scolaro <[email protected]>

poiana added release-note-none kind/cleanup dco-signoff: yes area/libsinsp size/XS labels Nov 5, 2024

poiana requested review from hbrueckner and incertum November 5, 2024 10:08

poiana added this to the 0.19.0 milestone Nov 5, 2024

FedeDP previously approved these changes Nov 5, 2024

View reviewed changes

poiana assigned FedeDP Nov 5, 2024

poiana added the lgtm label Nov 5, 2024

poiana added the approved label Nov 5, 2024

Andreagit97 previously approved these changes Nov 5, 2024

View reviewed changes

poiana assigned Andreagit97 Nov 5, 2024

therealbobo dismissed stale reviews from Andreagit97 and FedeDP via c74d31b November 6, 2024 14:31

poiana removed the lgtm label Nov 6, 2024

poiana requested review from Andreagit97 and FedeDP November 6, 2024 14:31

fix(libsinsp/test): fix async_key_value_source flakiness

50ffd82

Signed-off-by: Roberto Scolaro <[email protected]>

therealbobo force-pushed the therealbobo-misc branch from c74d31b to 50ffd82 Compare November 6, 2024 18:43

FedeDP reviewed Nov 7, 2024

View reviewed changes

jasondellaluce approved these changes Nov 7, 2024

View reviewed changes

poiana assigned jasondellaluce Nov 7, 2024

poiana added the lgtm label Nov 7, 2024

FedeDP approved these changes Nov 7, 2024

View reviewed changes

poiana merged commit dd8d508 into falcosecurity:master Nov 7, 2024
47 of 49 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(libsinsp): avoid useless allocation #2148

chore(libsinsp): avoid useless allocation #2148

therealbobo commented Nov 5, 2024

github-actions bot commented Nov 5, 2024 •

edited

Loading

codecov bot commented Nov 5, 2024 •

edited

Loading

FedeDP commented Nov 5, 2024

FedeDP commented Nov 5, 2024

FedeDP left a comment

poiana commented Nov 5, 2024

Andreagit97 left a comment

FedeDP Nov 7, 2024

therealbobo Nov 7, 2024

jasondellaluce left a comment

poiana commented Nov 7, 2024

FedeDP left a comment

poiana commented Nov 7, 2024

chore(libsinsp): avoid useless allocation #2148

chore(libsinsp): avoid useless allocation #2148

Conversation

therealbobo commented Nov 5, 2024

github-actions bot commented Nov 5, 2024 • edited Loading

Perf diff from master - unit tests

Heap diff from master - unit tests

Heap diff from master - scap file

Benchmarks diff from master

codecov bot commented Nov 5, 2024 • edited Loading

Codecov Report

FedeDP commented Nov 5, 2024

FedeDP commented Nov 5, 2024

FedeDP left a comment

Choose a reason for hiding this comment

poiana commented Nov 5, 2024

Andreagit97 left a comment

Choose a reason for hiding this comment

FedeDP Nov 7, 2024

Choose a reason for hiding this comment

therealbobo Nov 7, 2024

Choose a reason for hiding this comment

jasondellaluce left a comment

Choose a reason for hiding this comment

poiana commented Nov 7, 2024

FedeDP left a comment

Choose a reason for hiding this comment

poiana commented Nov 7, 2024

github-actions bot commented Nov 5, 2024 •

edited

Loading

codecov bot commented Nov 5, 2024 •

edited

Loading