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

[Plat-8841] feature flags order #1481

Merged
merged 1 commit into from
Sep 14, 2022
Merged

Conversation

kstenerud
Copy link
Contributor

@kstenerud kstenerud commented Sep 13, 2022

Goal

PLAT-8841: Keep ordering of feature flags, and improve performance of the feature flags store

Design

Replaced the array based feature flags store with a combined array and dictionary in order to speed up access.

Each dictionary entry maps to an index in the backing array, which contains the feature flag. Removing a feature flag leaves a hole in the array in order to avoid compaction costs. A forced compaction occurs only if there are more than 1000 holes.

The measured performance difference between this and a straight-up unordered dictionary is negligible (I've put in performance tests for easy comparison).

Testing

Updated tests to match the new expected feature flag ordering, and added some performance tests.

@github-actions
Copy link

github-actions bot commented Sep 13, 2022

Bugsnag.framework binary size increased by 2,760 bytes from 702,608 to 705,368

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  [ = ]       0  +716% +13.3Ki    [__LINKEDIT]
  +0.9% +2.00Ki  +0.9% +2.00Ki    __TEXT,__text
  +1.0% +1.53Ki  +1.0% +1.53Ki    String Table
  +0.9% +1.12Ki  +0.9% +1.12Ki    Symbol Table
  +1.0%    +360  +1.0%    +360    __DATA,__objc_const
  +2.4%     +80  +2.4%     +80    __DATA,__objc_data
  +0.3%     +56  +0.3%     +56    __TEXT,__objc_methname
  +1.1%     +32  +1.1%     +32    __TEXT,__unwind_info
  +1.4%     +24  +1.4%     +24    Rebase Info
  +1.1%     +23  +1.1%     +23    __TEXT,__objc_methtype
  +1.9%     +20  +1.9%     +20    __TEXT,__objc_classname
  +1.0%     +16  +1.0%     +16    Function Start Addresses
  +0.4%     +12  +0.4%     +12    __TEXT,__gcc_except_tab
  +2.4%      +8  +2.4%      +8    __DATA,__objc_classlist
  +1.3%      +8  +1.3%      +8    __DATA,__objc_classrefs
  +0.9%      +8  +0.9%      +8    __DATA,__objc_ivar
  +0.2%      +8  +0.2%      +8    __DATA,__objc_selrefs
  +3.3%      +8  +3.3%      +8    __DATA,__objc_superrefs
  -5.4%    -480  -3.1%    -480    [__DATA]
  -7.8% -2.14Ki  -7.8% -2.14Ki    [__TEXT]
  +0.4% +2.70Ki  +2.3% +16.0Ki    TOTAL

Generated by 🚫 Danger

@kstenerud kstenerud force-pushed the PLAT-8841-feature-flags-order branch 4 times, most recently from f45d05b to e9d7c0e Compare September 14, 2022 07:37
@kstenerud kstenerud marked this pull request as ready for review September 14, 2022 07:37
Copy link
Contributor

@nickdowell nickdowell left a comment

Choose a reason for hiding this comment

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

Implementation looks sound but currently too much is exposed in BugsnagInternals.h and I'm hoping BSG_OBJC_DIRECT_MEMBERS will help reduce the bloat (currently +3.4K 🙁)

Bugsnag/BugsnagInternals.h Outdated Show resolved Hide resolved
Bugsnag/Helpers/BSGFeatureFlagStore.m Outdated Show resolved Hide resolved
Bugsnag/Helpers/BSGFeatureFlagStore.m Outdated Show resolved Hide resolved
Bugsnag/Helpers/BSGFeatureFlagStore.m Show resolved Hide resolved
Bugsnag/Helpers/BSGFeatureFlagStore.m Outdated Show resolved Hide resolved
Bugsnag/Helpers/BSGFeatureFlagStore.m Outdated Show resolved Hide resolved
Bugsnag/Helpers/BSGFeatureFlagStore.m Show resolved Hide resolved
Bugsnag/Helpers/BSGFeatureFlagStore.m Show resolved Hide resolved
Bugsnag/Helpers/BSGFeatureFlagStore.m Outdated Show resolved Hide resolved
@kstenerud kstenerud force-pushed the PLAT-8841-feature-flags-order branch 5 times, most recently from 7d655d6 to 9664d2c Compare September 14, 2022 09:11
@kstenerud kstenerud merged commit 93ae884 into next Sep 14, 2022
@kstenerud kstenerud deleted the PLAT-8841-feature-flags-order branch September 14, 2022 12:10
@nickdowell nickdowell mentioned this pull request Sep 21, 2022
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.

2 participants