Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Don't use <regex> to reduce binary size #7095

Merged
merged 1 commit into from
Nov 18, 2016
Merged

Don't use <regex> to reduce binary size #7095

merged 1 commit into from
Nov 18, 2016

Conversation

kkaefer
Copy link
Member

@kkaefer kkaefer commented Nov 16, 2016

We're only using <regex> in one instance, for removing access_token=* from URLs when putting them into the cache. However, <regex> adds quite a bit to the binary size; I'm measuring these size reductions:

  • Android/arm-v7: 4,903,78 => 4,842,348 bytes, ~60KB reduction
  • macOS/x86_64: 3,955,184 => 3,860,600 bytes, ~95KB reduction
  • iOS/arm64: 3,012,356 => 2,961,728 bytes, ~50KB reduction

@mention-bot
Copy link

@kkaefer, thanks for your PR! By analyzing the history of the files in this pull request, we identified @mapsam, @jfirebaugh and @lucaswoj to be potential reviewers.

@mapsam
Copy link
Contributor

mapsam commented Nov 16, 2016

This was very specifically added to ensure other parameters are preserved, and only access token is removed. This is important for style-optimized vector tile requests that may have a &style= parameter in the tile URL. If the tests continue to pass, 👍 from me.

Copy link
Contributor

@jfirebaugh jfirebaugh left a comment

Choose a reason for hiding this comment

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

I'm fine with this, although we'll likely eventually re-add this dependency for mapbox/mapbox-gl-style-spec#233 / mapbox/mapbox-gl-style-spec#321.

Don't forget to remove the #include <regex> too.

@kkaefer
Copy link
Member Author

kkaefer commented Nov 17, 2016

@mapsam yup, the code just looks for access_token fields and removes them, while preserving all others.

@kkaefer kkaefer merged commit 6c91c3e into master Nov 18, 2016
@kkaefer kkaefer deleted the 7095-no-regex branch November 18, 2016 09:38
@mapsam
Copy link
Contributor

mapsam commented Nov 21, 2016

Thanks @kkaefer!

jfirebaugh added a commit that referenced this pull request Sep 11, 2018
jfirebaugh added a commit that referenced this pull request Sep 13, 2018
jfirebaugh added a commit that referenced this pull request Sep 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants