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

Cleanup mbgl/actor/mailbox* implementation for repetition in ensuring valid weakScheduler exists before usage #2733

Merged
merged 2 commits into from
Aug 26, 2024

Conversation

bilal614
Copy link
Contributor

Ensure that valid weakScheduler exists before using it to schedule. Remove repetition of weakScheduler guarantee such that it is captured in a method.

@bilal614
Copy link
Contributor Author

Relates to Issue #2732

@louwers louwers added cpp-core enhancement New feature or request labels Aug 19, 2024
Copy link

Bloaty Results (iOS) 🐋

Compared to main

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  -0.0%     -96  [ = ]       0    TOTAL

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

Copy link

Bloaty Results 🐋

Compared to main

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.0% +2.30Ki  +0.0%    +288    TOTAL

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

Compared to d387090 (legacy)

    FILE SIZE        VM SIZE    
 --------------  -------------- 
   +27% +31.0Mi  +420% +25.1Mi    TOTAL

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

Copy link

Benchmark Results ⚡

Benchmark                                                     Time             CPU      Time Old      Time New       CPU Old       CPU New
------------------------------------------------------------------------------------------------------------------------------------------
OVERALL_GEOMEAN                                            +0.0098         +0.0098             0             0             0             0

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

@bilal614
Copy link
Contributor Author

@mwilsnd: Could you please have a look at the changes for providing any review feedback. thanks

@bilal614
Copy link
Contributor Author

@louwers: The pipeline looks green. Is there any review feedback I can incorporate before the merge?

@louwers
Copy link
Collaborator

louwers commented Aug 26, 2024

Nope, let's give @mwilsnd one more day to look at it since he reworked this code recently.

Otherwise I'll merge it later today!

Copy link
Collaborator

@mwilsnd mwilsnd left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for consolidating this stuff!

@louwers louwers merged commit cf55873 into maplibre:main Aug 26, 2024
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cpp-core enhancement New feature or request
Projects
Development

Successfully merging this pull request may close these issues.

3 participants