refactor(libsinsp/threadinfo): export static fields via static method

[ekoops]

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:

This PR makes static_struct::define_static_field() static and accepting a field parameter pointer instead of a reference. It leverages this change to define an additional static method sinsp_threadinfo::get_static_fields() that can be used for the global initialization of s_threadinfo_static_fields in threadinfo.cpp. This is useful to avoid instantiating a sinsp_threadinfo object to initialize s_threadinfo_static_fields, a preliminary step needed before isolating the sinsp_thread_manager dependencies and passing them directly to its constructor.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

[gnosek]

/approve

[poiana]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ekoops, gnosek

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 [gnosek]

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

[github-actions]

Perf diff from master - unit tests

    19.76%     -0.64%  [.] sinsp_threadinfo::get_main_thread
     1.29%     -0.32%  [.] next
     9.37%     -0.31%  [.] sinsp_thread_manager::create_thread_dependencies
     1.35%     -0.29%  [.] user_group_updater::user_group_updater
     0.91%     -0.27%  [.] sinsp_evt::get_ts
     1.40%     +0.27%  [.] is_conversion_needed
     4.01%     +0.25%  [.] next_event_from_file
     8.66%     -0.22%  [.] std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release
     5.15%     +0.20%  [.] sinsp_evt::get_type
     0.83%     +0.19%  [.] libsinsp::sinsp_suppress::process_event

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.0251         +0.0251           143           147           143           147
BM_sinsp_split_median                                          +0.0275         +0.0276           143           147           143           147
BM_sinsp_split_stddev                                          +0.4271         +0.4276             2             2             2             2
BM_sinsp_split_cv                                              +0.3921         +0.3927             0             0             0             0
BM_sinsp_concatenate_paths_relative_path_mean                  -0.0638         -0.0638            58            55            58            55
BM_sinsp_concatenate_paths_relative_path_median                -0.0612         -0.0612            58            54            58            54
BM_sinsp_concatenate_paths_relative_path_stddev                -0.2641         -0.2639             1             1             1             1
BM_sinsp_concatenate_paths_relative_path_cv                    -0.2139         -0.2137             0             0             0             0
BM_sinsp_concatenate_paths_empty_path_mean                     -0.0840         -0.0840            26            24            26            24
BM_sinsp_concatenate_paths_empty_path_median                   -0.0860         -0.0860            26            24            26            24
BM_sinsp_concatenate_paths_empty_path_stddev                   +0.0614         +0.0613             0             0             0             0
BM_sinsp_concatenate_paths_empty_path_cv                       +0.1586         +0.1586             0             0             0             0
BM_sinsp_concatenate_paths_absolute_path_mean                  -0.1022         -0.1022            58            52            58            52
BM_sinsp_concatenate_paths_absolute_path_median                -0.1004         -0.1004            58            52            58            52
BM_sinsp_concatenate_paths_absolute_path_stddev                -0.2330         -0.2329             1             0             1             0
BM_sinsp_concatenate_paths_absolute_path_cv                    -0.1456         -0.1455             0             0             0             0

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.17%. Comparing base (3d1d4a9) to head (782ed13).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2296      +/-   ##
==========================================
+ Coverage   77.13%   77.17%   +0.04%     
==========================================
  Files         220      222       +2     
  Lines       30088    30132      +44     
  Branches     4598     4600       +2     
==========================================
+ Hits        23207    23255      +48     
+ Misses       6881     6877       -4     

Flag	Coverage Δ
libsinsp	77.17% <100.00%> (+0.04%)	⬆️

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.

[gnosek]

@ekoops I took a look at how offsetof manages not to hit ubsan and it turns out that at least in my stdlib headers, it's a compiler builtin:

/* Offset of member MEMBER in a struct of type TYPE. */
#define offsetof(TYPE, MEMBER) __builtin_offsetof (TYPE, MEMBER)

So I guess we'd have to explicitly pass the offset rather than a pointer inside a NULL-based struct. The downside is that offsetof doesn't have any type information. In my (unrelated) experiments, the best I came up with was a macro using decltype(T{}.FIELD) and offsetof(T, FIELD) but that doesn't help us a lot if the whole point of the exercise is to avoid a default constructor for sinsp_threadinfo :) Still, the default constructor would never actually be invoked -- maybe an unimplemented free function or static method that returns sinsp_threadinfo would be good enough for decltype?

[ekoops]

@ekoops I took a look at how offsetof manages not to hit ubsan and it turns out that at least in my stdlib headers, it's a compiler builtin:

/* Offset of member MEMBER in a struct of type TYPE. */
#define offsetof(TYPE, MEMBER) __builtin_offsetof (TYPE, MEMBER)

So I guess we'd have to explicitly pass the offset rather than a pointer inside a NULL-based struct. The downside is that offsetof doesn't have any type information. In my (unrelated) experiments, the best I came up with was a macro using decltype(T{}.FIELD) and offsetof(T, FIELD) but that doesn't help us a lot if the whole point of the exercise is to avoid a default constructor for sinsp_threadinfo :) Still, the default constructor would never actually be invoked -- maybe an unimplemented free function or static method that returns sinsp_threadinfo would be good enough for decltype?

Hi @gnosek , thank you for the help! I conducted similar experiments and approximately came out with the same conclusions. I'll still try to investigate the macro path, hoping there's a possible way to avoid creating and object. I'll also explore the other suggestion and update you