Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Benchmark app frame time improvements #1776

Merged

Conversation

alexcristici
Copy link
Collaborator

@alexcristici alexcristici commented Oct 18, 2023

Refactored benchmark app to use headless frontend for an accurate measurement of the frame time.

Make sure to set the Benchmark app on Release, without Debug executable set and disable Metal API validation:
Screenshot 2023-11-02 at 20 47 16
Screenshot 2023-11-02 at 20 47 06

@alexcristici alexcristici self-assigned this Oct 18, 2023
@github-actions
Copy link

github-actions bot commented Oct 18, 2023

Bloaty Results 🐋

Compared to main

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.0%    +848  +0.0%    +360    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-1776-compared-to-main.txt

Compared to d387090 (legacy)

    FILE SIZE        VM SIZE    
 --------------  -------------- 
   +19% +22.1Mi  +401% +24.0Mi    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-1776-compared-to-legacy.txt

@alexcristici alexcristici marked this pull request as ready for review November 2, 2023 20:04
Copy link
Collaborator

@sjg-wdw sjg-wdw left a comment

Choose a reason for hiding this comment

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

Looks reasonable, but someone with more familiarity with the MLN View should weigh in.

Copy link
Collaborator

@louwers louwers left a comment

Choose a reason for hiding this comment

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

Could you share some more details about why these changes were needed here, as well as your test results?

Copy link
Collaborator

@TimSylvester TimSylvester left a comment

Choose a reason for hiding this comment

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

On my iPad mini 5:

| boston | 3.2210 ms | 4.7509 ms |
| paris | 2.5287 ms | 12.5014 ms |
| paris2 | 2.1421 ms | 12.5297 ms |
| alps | 2.9439 ms | 21.9420 ms |
| us east | 2.0176 ms | 14.9323 ms |
| greater la | 2.2015 ms | 14.6801 ms |
| sf | 4.0980 ms | 18.4790 ms |
| oakland | 2.5990 ms | 8.9986 ms |
| germany | 3.4378 ms | 22.3297 ms |
Average frame encoding time: 2.7988 ms
Average frame rendering time: 14.5715 ms

@alexcristici
Copy link
Collaborator Author

alexcristici commented Nov 3, 2023

Could you share some more details about why these changes were needed here, as well as your test results?

Old implementation was rendering directly to the MTKView or on screen. Because of that, frame measurement was affected by waiting for new drawable to be available in the MTKView. Because the display is set to 60 fps, new drawable is available one time at 16.666 ms. Even though the encoding of the gpu commands would have taken less than 16.666 ms, the frame time would measure 16.66 ms because it was waiting for the new drawable.

In order have an accurate measurement and not being affected by this waiting time (when new drawable is available) I had to render the map to an offscreen texture, using the headless frontend. In this way I can render frame after frame without being limited by vsync of 60 fps and measure correctly the encoding time of the gpu command.

Latest results using refactored Benchmark app:
Screenshot 2023-11-02 at 23 05 37

@alexcristici alexcristici merged commit 8ad5f9c into maplibre:main Nov 3, 2023
37 checks passed
@alexcristici alexcristici deleted the benchmark-frame-time-improvements branch November 3, 2023 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants