Skip to content

Conversation

@Himess
Copy link

@Himess Himess commented Oct 28, 2025

Summary

This PR removes unused dependencies and eliminates redundant clone operations across the codebase, improving build times and runtime performance.

Changes

kms-connector:

  • serial_test from connector-utils (dev-dependency)

fhevm-engine/stress-test-generator:

  • aws-sdk-s3, humantime, regex, scheduler, semver, serial_test, test-harness, testcontainers, tfhe-worker, tonic-build, zkproof-worker

fhevm-engine/tfhe-worker:

  • actix-web, tonic-health, tonic-types, tonic-web

Redundant Clones Removed (11 total)

  • fhevm-engine-common: 4 unnecessary .clone() calls
  • scheduler: 3 unnecessary .clone() calls
  • gw-listener: 3 unnecessary .to_owned() calls
  • kms-worker: 1 unnecessary .to_string() call

Testing

image

- kms-connector/utils: remove unused serial_test
- fhevm-engine/stress-test-generator: remove 11 unused deps
- fhevm-engine/tfhe-worker: remove 4 unused deps (actix-web, tonic-*)

Detected by cargo-udeps
- Remove unnecessary .clone() and .to_owned() calls
- Improves performance by avoiding unnecessary allocations

Detected by clippy::redundant_clone
@Himess Himess requested a review from a team as a code owner October 28, 2025 16:30
@cla-bot cla-bot bot added the cla-signed label Oct 28, 2025
@rudy-6-4
Copy link
Contributor

Hi,
Thanks for improving the code base.
Can you activate clippy::redundant_clone so your work set the bar for everybody after and there is no regression ?
https://github.com/zama-ai/fhevm/blob/main/.github/workflows/coprocessor-cargo-clippy.yml#L85
Adding cargo-udeps maybe more complicated but if easy you can add it or share the commands you use so we add it quickly.

I am launching the workflows.

Copy link
Contributor

@rudy-6-4 rudy-6-4 left a comment

Choose a reason for hiding this comment

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

LGTM, waiting workflow execution to approve

@rudy-6-4
Copy link
Contributor

Note that you need to resolve the conflicts to be able to run the github workflows.

@Himess
Copy link
Author

Himess commented Oct 29, 2025

Hi, thanks for the review! @rudy-6-4
Sure , I will enable clippy::redundant_clone and also check cargo-udeps.
I will resolve the conflicts and fix the failing workflows as soon as I'm available.

Resolved conflicts in:
- coprocessor/fhevm-engine/gw-listener/src/aws_s3.rs
- coprocessor/fhevm-engine/gw-listener/src/gw_listener.rs
Add clippy::redundant_clone warning to prevent future regressions
as requested in code review.
@Himess
Copy link
Author

Himess commented Oct 29, 2025

@rudy-6-4

I've enabled clippy::redundant_clone in the CI workflow as requested. ✅

I've also resolved the merge conflicts with upstream/main. Could you please re-run the workflows when you have a chance?

For finding all clone-related issues locally, I used:

cargo clippy --all-targets -- -W clippy::redundant_clone -W clippy::clone_on_copy -W clippy::unnecessary_to_owned

And for unused dependencies:

cargo +nightly udeps --all-targets

If you'd like to add cargo-udeps to CI or enable additional clippy lints, I'm happy to help - just let me know!

@Himess Himess requested a review from rudy-6-4 October 29, 2025 17:39
@Himess Himess marked this pull request as draft October 30, 2025 23:15
@Himess
Copy link
Author

Himess commented Oct 30, 2025

@rudy-6-4
Updated Cargo.lock after dependency removals.
Converting to draft temporarily - found a compilation issue after merging with main (upstream changed MetaParameters struct). Will fix and mark as ready for review shortly.

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.

2 participants