Skip to content

Conversation

adrian-kong
Copy link
Contributor

@adrian-kong adrian-kong commented Oct 31, 2022

@adrian-kong adrian-kong changed the title qml optimizations qml optimizations [CPP-818] Oct 31, 2022
@adrian-kong
Copy link
Contributor Author

front end benchmarks seem to run a bit slower compared to main branch

@silverjam silverjam requested a review from keithel-qt October 31, 2022 15:18
@silverjam
Copy link
Contributor

@keithel-qt can you review please?

for (var idx in Constants.advancedImu.lineColors) {
if (lineLegendRepeaterRows.itemAt(idx))
lineLegendRepeaterRows.itemAt(idx).children[0].color = Constants.advancedImu.lineColors[idx];
let imuLineColors = Constants.advancedImu.lineColors;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not that I have any way to validate my hunch but I imagine this originally would have only counted as a single "resolve" event.

@john-michaelburke
Copy link
Collaborator

front end benchmarks seem to run a bit slower compared to main branch

Two thoughts:

  • All of the other slowdowns in the app may greatly overshadow any minor improvements and may already have a pretty significant variance in wall time.
  • If you really wanted to assess the performance of this change, I think you could rerun the frontend benchmarks ~5-10 times and compare the average before and after this change. Or you could go even further and do some profiling like what Keith has been doing.

@adrian-kong
Copy link
Contributor Author

manually benchmarking, this seems to do inconsistent ±1-2% cpu utilization, seems too little to actually know if this is improving

@silverjam
Copy link
Contributor

Create a test tag to trial this on a low power machine: https://github.com/swift-nav/swift-toolbox/releases/tag/v4.1.4-qmlopt (still building)

@st-swift
Copy link

st-swift commented Nov 4, 2022

Screenshot 2022-11-04 125043

Before

@st-swift
Copy link

st-swift commented Nov 4, 2022

Screenshot 2022-11-04 130749

After

Copy link
Contributor

@silverjam silverjam left a comment

Choose a reason for hiding this comment

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

Eyeballing things there seems to be some improvement (maybe) but definitely doesn't seem to hurt anything

@silverjam silverjam enabled auto-merge (squash) November 4, 2022 20:26
@silverjam silverjam merged commit be2f26f into main Nov 4, 2022
@silverjam silverjam deleted the adrian/qml_opt branch November 4, 2022 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants