Skip to content

Conversation

yann-lty
Copy link
Member

@yann-lty yann-lty commented Feb 5, 2025

Description

This PR adresses performance issues when loading large project files (in UI mode), introduced as part of #2586.
It changes the way a Node is notified of an Attribute value change, which has a great impact on Attribute creation time.
The figures below shows the before and after for a project with a large number of attributes.

Before
meshroom_connect_lambda
Overall attribute creation time: 146s

After
meshroom_connect_slot
Overall attribute creation time: 1.24s

Features list

  • Improve large project file loading performance.

Implementation remarks

The notification of attribute value change to the owning node has been reworked to use a Signal/Slot connection, rather than a Signal/lambda.
This leads to much better performance, while preserving the same behavior.

…Slot

Connecting the valueChanged signal to a lambda causes performance
issues with a large number of attributes when using the Pyside backend.
Connecting that signal to an intermediate Slot part of the Attribute class
improves performance dramatically.
Note: This has no comparable impact on the standalone backend.
Copy link

codecov bot commented Feb 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.96%. Comparing base (910736b) to head (37a813e).
⚠️ Report is 420 commits behind head on develop.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2665      +/-   ##
===========================================
+ Coverage    69.95%   69.96%   +0.01%     
===========================================
  Files          122      122              
  Lines         7106     7109       +3     
===========================================
+ Hits          4971     4974       +3     
  Misses        2135     2135              

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@yann-lty yann-lty marked this pull request as ready for review February 5, 2025 12:02
@yann-lty yann-lty requested a review from fabiencastan February 5, 2025 15:03
@yann-lty yann-lty added this to the Meshroom 2025.1.0 milestone Feb 5, 2025
@yann-lty yann-lty requested a review from waaake February 6, 2025 11:16
@servantftransperfect servantftransperfect merged commit ebf2270 into develop Feb 6, 2025
5 checks passed
@servantftransperfect servantftransperfect deleted the fix/attrValueChangedConnection branch February 6, 2025 14:55
@fabiencastan fabiencastan added the feature new feature (proposed as PR or issue planned by dev) label Aug 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature new feature (proposed as PR or issue planned by dev)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants