Repository navigation
CURA-13218 early profiling results - #2401
Open
Erwan MATHIEU (wawanbreton) wants to merge 9 commits into
Open
Erwan MATHIEU (wawanbreton) wants to merge 9 commits into
Erwan MATHIEU (wawanbreton) wants to merge 9 commits into
Conversation
Bounding-box comparisons are faster, so use as an early-out. early results from CURA-13218
Don't compare by writing to strings (first) anymore -- it's quite slow. early results from CURA-13218
CURA-13218 When doing addressing of values in an array in a safe environment, i.e. known array size and fixed index, it is more efficient to use the [] operator which saves the bounds checking.
CURA-13218 This may already be optimized by the compiler, but it doesn't hurt
Copilot started reviewing on behalf of
Erwan MATHIEU (wawanbreton)
October 8, 2026 09:38
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unordered AABB endpoints can miss intersections, while truncated fan-speed comparison can suppress required commands.
2 open findings
What changed in this PR
Applies profiling-driven optimizations to geometry and G-code generation.
Changes:
- Reduces matrix bounds checks and parameter copying.
- Adds segment AABB filtering and fan-speed comparison optimization.
- Corrects progress-stage naming.
| File | Description |
|---|---|
src/utils/polygonUtils.cpp |
Avoids point and matrix copies. |
src/geometry/Shape.cpp |
Adds AABB intersection prefiltering. |
src/geometry/PointMatrix.cpp |
Uses unchecked fixed-index access. |
src/gcode_export/gcodeExport.cpp |
Avoids string-based fan-speed comparison. |
include/utils/string.h |
Adds precision-aware comparison. |
include/utils/polygonUtils.h |
Updates and restricts collision helpers. |
include/progress/Progress.h |
Adds the painting stage name. |
include/geometry/Point3Matrix.h |
Optimizes fixed-index matrix operations. |
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
part of code review for CURA-13218 Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
Test Results31 tests 31 ✅ 4s ⏱️ Results for commit f2b9cbc. ♻️ This comment has been updated with latest results. |
In context, the numbers are being printed to gcode (and it's also just better, as otherwise you would have numbers that are arbitrarily close that don't match, but do match up to the entire range, even though another one would be closer) -- fmod truncated, and that's justs not what's needed right here (I might've thought that print also did that?). We just have the 'round' as a function though, so why not use that. done as part of CURA-13218
Contributor
Author
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'C++ Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
| Benchmark suite | Current: f2b9cbc | Previous: 6b7e1bb32719abebf553cfeff08229e92a11e728 | Ratio |
|---|---|---|---|
InfillTest/Infill_generate_connect/1/400 |
0.343145001357333 ms/iter |
0.20599059913879397 ms/iter |
1.67 |
InfillTest/Infill_generate_connect/0/400 |
5.731959569342786 ms/iter |
3.1839650456351 ms/iter |
1.80 |
InfillTest/Infill_generate_connect/1/800 |
44.70271543333789 ms/iter |
26.048666588234056 ms/iter |
1.72 |
InfillTest/Infill_generate_connect/0/800 |
150.1587147500345 ms/iter |
85.37365570000475 ms/iter |
1.76 |
InfillTest/Infill_generate_connect/1/1200 |
453.8424796667944 ms/iter |
237.75037919995157 ms/iter |
1.91 |
WallTestFixture/generateWalls/15 |
20.26857502499979 ms/iter |
13.330198688525671 ms/iter |
1.52 |
WallTestFixture/InsetOrderOptimizer_getRegionOrder/3 |
0.010476753748377742 ms/iter |
0.006454896409386443 ms/iter |
1.62 |
WallTestFixture/InsetOrderOptimizer_getRegionOrder/15 |
0.05633017073551842 ms/iter |
0.03514012285814793 ms/iter |
1.60 |
WallTestFixture/InsetOrderOptimizer_getRegionOrder/9999 |
0.1645095400893511 ms/iter |
0.10083142489146339 ms/iter |
1.63 |
WallTestFixture/InsetOrderOptimizer_getInsetOrder/3 |
0.0010384048817348925 ms/iter |
0.0006213279069972942 ms/iter |
1.67 |
WallTestFixture/InsetOrderOptimizer_getInsetOrder/15 |
0.005285448094300676 ms/iter |
0.0032259466336103703 ms/iter |
1.64 |
WallTestFixture/InsetOrderOptimizer_getInsetOrder/9999 |
0.011450507659833301 ms/iter |
0.0076085670566758265 ms/iter |
1.50 |
HolesWallTestFixture/InsetOrderOptimizer_getRegionOrder/3 |
0.004412526900979489 ms/iter |
0.0026305655630471337 ms/iter |
1.68 |
HolesWallTestFixture/InsetOrderOptimizer_getRegionOrder/15 |
0.02226698957367099 ms/iter |
0.013478111510376974 ms/iter |
1.65 |
HolesWallTestFixture/InsetOrderOptimizer_getRegionOrder/9999 |
0.2651614591604117 ms/iter |
0.16757784821010394 ms/iter |
1.58 |
HolesWallTestFixture/InsetOrderOptimizer_getInsetOrder/3 |
0.0002519151543105202 ms/iter |
0.00014716848055045274 ms/iter |
1.71 |
HolesWallTestFixture/InsetOrderOptimizer_getInsetOrder/15 |
0.001047489680085581 ms/iter |
0.0006168532438831372 ms/iter |
1.70 |
HolesWallTestFixture/InsetOrderOptimizer_getInsetOrder/9999 |
0.011084526213535067 ms/iter |
0.006639665026949777 ms/iter |
1.67 |
SimplifyTestFixture/simplify_slot_noplugin |
3.740274771547497 ns/iter |
1.5467188415681639 ns/iter |
2.42 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: Jelle Spijker (@jellespijker) Erwan MATHIEU (@wawanbreton) Casper Lamboo (@casperlamboo) HellAholic
Member
|
A performance alert on this specific PR has a certain irony to it 😓 |
Contributor
Author
Yeah this thing keeps randomly popping up, maybe we should do something about it because now I just got used to ignore it. |
Member
I agree, I'm also ignoring it for the same reason. I'd prefer to fix whatever's causing it to misbehave, but if we can't we should just disable it. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
During investigation on the engine performance profiling, Remco Burema (@rburema) and I we found a few quick wins that we can apply to speedup performance. Most of them are mostly sanity and will probably not provide a significant performance change, but it is always good to take.
CURA-13218