Skip to content

Conversation

ekoops
Copy link
Contributor

@ekoops ekoops commented Apr 16, 2025

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 reduces fdtable's params resources waste by moving them into a separate struct provided at construction phase and shared among all fdtable instances.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

feat(libsinsp/userspace)!: reduce fdtable's params resources waste

@ekoops ekoops changed the title feat(libsinsp/userspace)!: reduce fdtable's params resources waste wip: feat(libsinsp/userspace)!: reduce fdtable's params resources waste Apr 16, 2025
@poiana poiana requested review from hbrueckner and leogr April 16, 2025 08:03
@ekoops ekoops changed the title wip: feat(libsinsp/userspace)!: reduce fdtable's params resources waste feat(libsinsp/userspace)!: reduce fdtable's params resources waste Apr 16, 2025
@ekoops ekoops force-pushed the ekoops/fdtable-ctor-params branch 2 times, most recently from f4f8a2a to a0428ef Compare April 16, 2025 08:52
Copy link

codecov bot commented Apr 16, 2025

Codecov Report

Attention: Patch coverage is 86.66667% with 4 lines in your changes missing coverage. Please review.

Project coverage is 77.03%. Comparing base (dd92939) to head (5b54ad0).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
userspace/libsinsp/fdtable.cpp 90.00% 2 Missing ⚠️
userspace/libsinsp/fdtable.h 0.00% 1 Missing ⚠️
userspace/libsinsp/sinsp.cpp 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2352      +/-   ##
==========================================
- Coverage   77.05%   77.03%   -0.02%     
==========================================
  Files         228      228              
  Lines       30285    30271      -14     
  Branches     4635     4636       +1     
==========================================
- Hits        23335    23320      -15     
- Misses       6950     6951       +1     
Flag Coverage Δ
libsinsp 77.03% <86.66%> (-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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Reduce fdtable's params resources waste by moving them into a
separate struct provided at construction phase and shared among all
fdtable instances.

BREAKING CHANGE: update `sinsp_fdtable` constructor

Signed-off-by: Leonardo Di Giovanna <leonardodigiovanna1@gmail.com>
@ekoops ekoops force-pushed the ekoops/fdtable-ctor-params branch from a0428ef to 5b54ad0 Compare April 16, 2025 09:04
@FedeDP
Copy link
Contributor

FedeDP commented Apr 16, 2025

/milestone 0.21.0

@poiana poiana added this to the 0.21.0 milestone Apr 16, 2025
Copy link
Contributor

@FedeDP FedeDP 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 Apr 16, 2025

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

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

@poiana
Copy link
Contributor

poiana commented Apr 16, 2025

LGTM label has been added.

Git tree hash: cb2502a86931f3151b24f21b447efb9a3cc3687c

@github-project-automation github-project-automation bot moved this from Todo to In progress in Falco Roadmap Apr 16, 2025
@poiana poiana merged commit ab7eceb into falcosecurity:master Apr 16, 2025
46 of 47 checks passed
@github-project-automation github-project-automation bot moved this from In progress to Done in Falco Roadmap Apr 16, 2025
@ekoops ekoops deleted the ekoops/fdtable-ctor-params branch April 16, 2025 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants