Skip to content

Gate regex lookahead in cmake behind compile flag #59

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

Merged
merged 2 commits into from
Apr 24, 2025

Conversation

jackzhxng
Copy link
Contributor

Make the pcre2 fallback optional to offer library size savings

cmake -DSUPPORT_REGEX_LOOKAHEAD=ON -DTOKENIZERS_BUILD_TOOLS=ON . -Bbuild && cmake --build build -j9
stat -f "%z %N" build/libtokenizers.a
6394880 build/libtokenizers.a

cmake -DTOKENIZERS_BUILD_TOOLS=ON . -Bbuild && cmake --build build -j9
stat -f "%z %N" build/libtokenizers.a 
6311960 build/libtokenizers.a

# Difference of 6394880 - 6311960 = 82920 bytes

@jackzhxng jackzhxng requested a review from larryliu0820 April 23, 2025 19:18
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Apr 23, 2025
@facebook-github-bot
Copy link
Contributor

@jackzhxng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@jackzhxng jackzhxng force-pushed the jz/size-regression branch from 42f2f1e to 7e8c212 Compare April 23, 2025 19:25
Summary:
Make the pcre2 fallback optional to offer library size savings

```
cmake -DSUPPORT_REGEX_LOOKAHEAD=ON -DTOKENIZERS_BUILD_TOOLS=ON . -Bbuild && cmake --build build -j9
stat -f "%z %N" build/libtokenizers.a
6394880 build/libtokenizers.a

cmake -DTOKENIZERS_BUILD_TOOLS=ON . -Bbuild && cmake --build build -j9
stat -f "%z %N" build/libtokenizers.a 
6311960 build/libtokenizers.a

# Difference of 6394880 - 6311960 = 82920 bytes
```

BSB results on IOS `messenger-distribution`:
```
Changes for variation [arm64 + 3x assets]:
Compressed  : -80.4 KiB (-0.12%) => 64.92 MiB
Uncompressed: -235.2 KiB (-0.17%) => 138.98 MiB 
```

Compared to app size regression reported from 222011767:
```
uncompressed_delta_size_arm64@3x: change of 233058 (0.16%) exceeds threshold of 40960
```


Differential Revision: D73530475

Pulled By: jackzhxng
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D73530475

@facebook-github-bot
Copy link
Contributor

@jackzhxng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot facebook-github-bot merged commit ffd4749 into main Apr 24, 2025
5 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot. fb-exported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants