Add regex unit tests and enable shared linkage in fbcode #78

larryliu0820 · 2025-05-26T08:26:16Z

Summary:

As titled. We can't use weak symbol in fbcode since by default libraries are built in shared libraries and weak symbol doesn't work well with shared libraries.

Basically the dynamic linker will resolve to the first definition it finds, so strong symbol will be ignored.

There's an environment variable LD_DYNAMIC_WEAK to let ld fallback to the old behavior which respects the strong symbol but that's non-standard and support can be dropped.

See https://man7.org/linux/man-pages/man8/ld.so.8.html

As a result, this PR changes the pattern to be using static initializer to override the create_fallback_regex() function.

                                                                               
                                                                               
                                                                               
                          regex_lookahead.so                                   
                         ┌───────────────────────────────────────────────────┐ 
                         │  ┌────────────────────────────────────┐┌─────────┐│ 
                         │  │   override_regex_fn(fallback_fn)   ││         ││ 
 ┌─────────────┐         │  │                                    ││         ││ 
 │             │         │  │          regex_lookahead.cpp       ││  pcre2  ││ 
 │             │         │  │                                    ││         ││ 
 │             │         │  │                                    ││         ││ 
 │ application ┼─────────►  └────────────────────────────────────┘└─────────┘│ 
 │             │         └──────────────────────────┬────────────────────────┘ 
 │             │                                    │                          
 │             │                                    │                          
 │             │                                    │link to                   
 └─────┬───────┘                                    │                          
       │                  regex.so                  │                          
       │                 ┌──────────────────────────▼────────────────────────┐ 
       │                 │  ┌───────────────┐   ┌──────────────────────────┐ │ 
       │                 │  │               │   │    regex_fn = default    │ │ 
       │                 │  │               │   │    override_regex_fn()   │ │ 
       └─────────────────►  │   regex.h     │   │                          │ │ 
                         │  │               │   │        regex.cpp         │ │ 
                         │  │               │   │                          │ │ 
                         │  │               │   │                          │ │ 
                         │  └───────────────┘   └──────────────────────────┘ │ 
                         └───────────────────────────────────────────────────┘

With this setup, an application can link to regex.so (which doesn't have all the pcre2 symbols and thus having a smaller binary size). If the application wants to add lookahead support, they can additionally link to regex_lookahead.so and override the create_fallback_regex() function.

Differential Revision: D75391108

facebook-github-bot · 2025-05-26T08:26:41Z

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

Summary: As titled. After some investigation it looks like we need `--dynamic-list` to expose the weak symbol `create_fallback_regex` in `regex.so`. Without `--dynamic-list`, linker will only look for locally defined symbol (the weak implementation) and ignore the strong implementation. This way, when linking `regex_lookahead.so` the exported dynamic symbol can be bound to the strong implementation in `regex_lookahead.cpp`. This makes the unit tests work in fbcode, where by default we are using `mode/dev` which links everything in shared linkage. If using `mode/opt` we don't need `--dynamic-list` because the linking order doesn't matter. Differential Revision: D75391108

Summary: Pull Request resolved: #78 As titled. After some investigation it looks like we need `--dynamic-list` to expose the weak symbol `create_fallback_regex` in `regex.so`. Without `--dynamic-list`, linker will only look for locally defined symbol (the weak implementation) and ignore the strong implementation. This way, when linking `regex_lookahead.so` the exported dynamic symbol can be bound to the strong implementation in `regex_lookahead.cpp`. This makes the unit tests work in fbcode, where by default we are using `mode/dev` which links everything in shared linkage. If using `mode/opt` we don't need `--dynamic-list` because the linking order doesn't matter. Differential Revision: D75391108

facebook-github-bot · 2025-05-26T08:35:43Z

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

Summary: As titled. After some investigation it looks like we need `--dynamic-list` to expose the weak symbol `create_fallback_regex` in `regex.so`. Without `--dynamic-list`, linker will only look for locally defined symbol (the weak implementation) and ignore the strong implementation. This way, when linking `regex_lookahead.so` the exported dynamic symbol can be bound to the strong implementation in `regex_lookahead.cpp`. This makes the unit tests work in fbcode, where by default we are using `mode/dev` which links everything in shared linkage. If using `mode/opt` we don't need `--dynamic-list` because the linking order doesn't matter. Differential Revision: D75391108

facebook-github-bot · 2025-05-27T18:55:04Z

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

Summary: Pull Request resolved: #78 As titled. After some investigation it looks like we need `--dynamic-list` to expose the weak symbol `create_fallback_regex` in `regex.so`. Without `--dynamic-list`, linker will only look for locally defined symbol (the weak implementation) and ignore the strong implementation. This way, when linking `regex_lookahead.so` the exported dynamic symbol can be bound to the strong implementation in `regex_lookahead.cpp`. This makes the unit tests work in fbcode, where by default we are using `mode/dev` which links everything in shared linkage. If using `mode/opt` we don't need `--dynamic-list` because the linking order doesn't matter. Differential Revision: D75391108

Summary: As titled. We can't use weak symbol in fbcode since by default libraries are built in shared libraries and weak symbol doesn't work well with shared libraries. Basically the dynamic linker will resolve to the first definition it finds, so strong symbol will be ignored. There's an environment variable `LD_DYNAMIC_WEAK` to let `ld` fallback to the old behavior which respects the strong symbol but that's non-standard and support can be dropped. See https://man7.org/linux/man-pages/man8/ld.so.8.html As a result, this PR changes the pattern to be using static initializer to override the `create_fallback_regex()` function. ``` regex_lookahead.so ┌───────────────────────────────────────────────────┐ │ ┌────────────────────────────────────┐┌─────────┐│ │ │ override_regex_fn(fallback_fn) ││ ││ ┌─────────────┐ │ │ ││ ││ │ │ │ │ regex_lookahead.cpp ││ pcre2 ││ │ │ │ │ ││ ││ │ │ │ │ ││ ││ │ application ┼─────────► └────────────────────────────────────┘└─────────┘│ │ │ └──────────────────────────┬────────────────────────┘ │ │ │ │ │ │ │ │ │link to └─────┬───────┘ │ │ regex.so │ │ ┌──────────────────────────▼────────────────────────┐ │ │ ┌───────────────┐ ┌──────────────────────────┐ │ │ │ │ │ │ regex_fn = default │ │ │ │ │ │ │ override_regex_fn() │ │ └─────────────────► │ regex.h │ │ │ │ │ │ │ │ regex.cpp │ │ │ │ │ │ │ │ │ │ │ │ │ │ │ └───────────────┘ └──────────────────────────┘ │ └───────────────────────────────────────────────────┘ ``` With this setup, an application can link to `regex.so` (which doesn't have all the pcre2 symbols and thus having a smaller binary size). If the application wants to add lookahead support, they can additionally link to `regex_lookahead.so` and override the `create_fallback_regex()` function. Differential Revision: D75391108

facebook-github-bot · 2025-05-28T04:43:43Z

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

Summary: As titled. We can't use weak symbol in fbcode since by default libraries are built in shared libraries and weak symbol doesn't work well with shared libraries. Basically the dynamic linker will resolve to the first definition it finds, so strong symbol will be ignored. There's an environment variable `LD_DYNAMIC_WEAK` to let `ld` fallback to the old behavior which respects the strong symbol but that's non-standard and support can be dropped. See https://man7.org/linux/man-pages/man8/ld.so.8.html As a result, this PR changes the pattern to be using static initializer to override the `create_fallback_regex()` function. ``` regex_lookahead.so ┌───────────────────────────────────────────────────┐ │ ┌────────────────────────────────────┐┌─────────┐│ │ │ override_regex_fn(fallback_fn) ││ ││ ┌─────────────┐ │ │ ││ ││ │ │ │ │ regex_lookahead.cpp ││ pcre2 ││ │ │ │ │ ││ ││ │ │ │ │ ││ ││ │ application ┼─────────► └────────────────────────────────────┘└─────────┘│ │ │ └──────────────────────────┬────────────────────────┘ │ │ │ │ │ │ │ │ │link to └─────┬───────┘ │ │ regex.so │ │ ┌──────────────────────────▼────────────────────────┐ │ │ ┌───────────────┐ ┌──────────────────────────┐ │ │ │ │ │ │ regex_fn = default │ │ │ │ │ │ │ override_regex_fn() │ │ └─────────────────► │ regex.h │ │ │ │ │ │ │ │ regex.cpp │ │ │ │ │ │ │ │ │ │ │ │ │ │ │ └───────────────┘ └──────────────────────────┘ │ └───────────────────────────────────────────────────┘ ``` With this setup, an application can link to `regex.so` (which doesn't have all the pcre2 symbols and thus having a smaller binary size). If the application wants to add lookahead support, they can additionally link to `regex_lookahead.so` and override the `create_fallback_regex()` function. Differential Revision: D75391108

facebook-github-bot · 2025-05-28T17:54:11Z

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

Summary: Pull Request resolved: #78 As titled. We can't use weak symbol in fbcode since by default libraries are built in shared libraries and weak symbol doesn't work well with shared libraries. Basically the dynamic linker will resolve to the first definition it finds, so strong symbol will be ignored. There's an environment variable `LD_DYNAMIC_WEAK` to let `ld` fallback to the old behavior which respects the strong symbol but that's non-standard and support can be dropped. See https://man7.org/linux/man-pages/man8/ld.so.8.html As a result, this PR changes the pattern to be using static initializer to override the `create_fallback_regex()` function. ``` regex_lookahead.so ┌───────────────────────────────────────────────────┐ │ ┌────────────────────────────────────┐┌─────────┐│ │ │ override_regex_fn(fallback_fn) ││ ││ ┌─────────────┐ │ │ ││ ││ │ │ │ │ regex_lookahead.cpp ││ pcre2 ││ │ │ │ │ ││ ││ │ │ │ │ ││ ││ │ application ┼─────────► └────────────────────────────────────┘└─────────┘│ │ │ └──────────────────────────┬────────────────────────┘ │ │ │ │ │ │ │ │ │link to └─────┬───────┘ │ │ regex.so │ │ ┌──────────────────────────▼────────────────────────┐ │ │ ┌───────────────┐ ┌──────────────────────────┐ │ │ │ │ │ │ regex_fn = default │ │ │ │ │ │ │ override_regex_fn() │ │ └─────────────────► │ regex.h │ │ │ │ │ │ │ │ regex.cpp │ │ │ │ │ │ │ │ │ │ │ │ │ │ │ └───────────────┘ └──────────────────────────┘ │ └───────────────────────────────────────────────────┘ ``` With this setup, an application can link to `regex.so` (which doesn't have all the pcre2 symbols and thus having a smaller binary size). If the application wants to add lookahead support, they can additionally link to `regex_lookahead.so` and override the `create_fallback_regex()` function. Differential Revision: D75391108

Summary: As titled. We can't use weak symbol in fbcode since by default libraries are built in shared libraries and weak symbol doesn't work well with shared libraries. Basically the dynamic linker will resolve to the first definition it finds, so strong symbol will be ignored. There's an environment variable `LD_DYNAMIC_WEAK` to let `ld` fallback to the old behavior which respects the strong symbol but that's non-standard and support can be dropped. See https://man7.org/linux/man-pages/man8/ld.so.8.html As a result, this PR changes the pattern to be using static initializer to override the `create_fallback_regex()` function. ``` regex_lookahead.so ┌───────────────────────────────────────────────────┐ │ ┌────────────────────────────────────┐┌─────────┐│ │ │ override_regex_fn(fallback_fn) ││ ││ ┌─────────────┐ │ │ ││ ││ │ │ │ │ regex_lookahead.cpp ││ pcre2 ││ │ │ │ │ ││ ││ │ │ │ │ ││ ││ │ application ┼─────────► └────────────────────────────────────┘└─────────┘│ │ │ └──────────────────────────┬────────────────────────┘ │ │ │ │ │ │ │ │ │link to └─────┬───────┘ │ │ regex.so │ │ ┌──────────────────────────▼────────────────────────┐ │ │ ┌───────────────┐ ┌──────────────────────────┐ │ │ │ │ │ │ regex_fn = default │ │ │ │ │ │ │ override_regex_fn() │ │ └─────────────────► │ regex.h │ │ │ │ │ │ │ │ regex.cpp │ │ │ │ │ │ │ │ │ │ │ │ │ │ │ └───────────────┘ └──────────────────────────┘ │ └───────────────────────────────────────────────────┘ ``` With this setup, an application can link to `regex.so` (which doesn't have all the pcre2 symbols and thus having a smaller binary size). If the application wants to add lookahead support, they can additionally link to `regex_lookahead.so` and override the `create_fallback_regex()` function. Differential Revision: D75391108

facebook-github-bot · 2025-05-28T19:03:15Z

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

jackzhxng

LGTM, few nits. Thanks for the detailed PR description and diagram

targets.bzl

src/std_regex.cpp

src/regex.cpp

include/pytorch/tokenizers/pcre2_regex.h

Summary: As titled. We can't use weak symbol in fbcode since by default libraries are built in shared libraries and weak symbol doesn't work well with shared libraries. Basically the dynamic linker will resolve to the first definition it finds, so strong symbol will be ignored. There's an environment variable `LD_DYNAMIC_WEAK` to let `ld` fallback to the old behavior which respects the strong symbol but that's non-standard and support can be dropped. See https://man7.org/linux/man-pages/man8/ld.so.8.html As a result, this PR changes the pattern to be using static initializer to override the `create_fallback_regex()` function. ``` regex_lookahead.so ┌───────────────────────────────────────────────────┐ │ ┌────────────────────────────────────┐┌─────────┐│ │ │ override_regex_fn(fallback_fn) ││ ││ ┌─────────────┐ │ │ ││ ││ │ │ │ │ regex_lookahead.cpp ││ pcre2 ││ │ │ │ │ ││ ││ │ │ │ │ ││ ││ │ application ┼─────────► └────────────────────────────────────┘└─────────┘│ │ │ └──────────────────────────┬────────────────────────┘ │ │ │ │ │ │ │ │ │link to └─────┬───────┘ │ │ regex.so │ │ ┌──────────────────────────▼────────────────────────┐ │ │ ┌───────────────┐ ┌──────────────────────────┐ │ │ │ │ │ │ regex_fn = default │ │ │ │ │ │ │ override_regex_fn() │ │ └─────────────────► │ regex.h │ │ │ │ │ │ │ │ regex.cpp │ │ │ │ │ │ │ │ │ │ │ │ │ │ │ └───────────────┘ └──────────────────────────┘ │ └───────────────────────────────────────────────────┘ ``` With this setup, an application can link to `regex.so` (which doesn't have all the pcre2 symbols and thus having a smaller binary size). If the application wants to add lookahead support, they can additionally link to `regex_lookahead.so` and override the `create_fallback_regex()` function. Reviewed By: billmguo Differential Revision: D75391108

facebook-github-bot · 2025-05-28T19:56:55Z

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

facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label May 26, 2025

facebook-github-bot added the fb-exported label May 26, 2025

larryliu0820 force-pushed the export-D75391108 branch 2 times, most recently from fb67eae to ef519a2 Compare May 26, 2025 08:35

larryliu0820 force-pushed the export-D75391108 branch from ef519a2 to 3dad9b2 Compare May 27, 2025 18:51

larryliu0820 force-pushed the export-D75391108 branch from 3dad9b2 to 5bae8eb Compare May 27, 2025 18:55

facebook-github-bot force-pushed the export-D75391108 branch from 5bae8eb to 2e6320c Compare May 28, 2025 04:43

larryliu0820 requested a review from jackzhxng May 28, 2025 04:44

larryliu0820 force-pushed the export-D75391108 branch from 2e6320c to 317a888 Compare May 28, 2025 17:51

larryliu0820 force-pushed the export-D75391108 branch from 317a888 to e3f4c42 Compare May 28, 2025 17:54

facebook-github-bot force-pushed the export-D75391108 branch from e3f4c42 to a608a51 Compare May 28, 2025 19:03

billmguo approved these changes May 28, 2025

View reviewed changes

jackzhxng approved these changes May 28, 2025

View reviewed changes

targets.bzl Show resolved Hide resolved

src/std_regex.cpp Show resolved Hide resolved

src/regex.cpp Outdated Show resolved Hide resolved

include/pytorch/tokenizers/pcre2_regex.h Show resolved Hide resolved

facebook-github-bot force-pushed the export-D75391108 branch from a608a51 to 346b017 Compare May 28, 2025 19:56

facebook-github-bot merged commit ea21dc7 into main May 29, 2025
4 of 6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add regex unit tests and enable shared linkage in fbcode #78

Add regex unit tests and enable shared linkage in fbcode #78

Uh oh!

larryliu0820 commented May 26, 2025 •

edited

Loading

Uh oh!

facebook-github-bot commented May 26, 2025

Uh oh!

facebook-github-bot commented May 26, 2025

Uh oh!

facebook-github-bot commented May 27, 2025

Uh oh!

facebook-github-bot commented May 28, 2025

Uh oh!

facebook-github-bot commented May 28, 2025

Uh oh!

facebook-github-bot commented May 28, 2025

Uh oh!

jackzhxng left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

facebook-github-bot commented May 28, 2025

Uh oh!

Uh oh!

Uh oh!

Add regex unit tests and enable shared linkage in fbcode #78

Add regex unit tests and enable shared linkage in fbcode #78

Uh oh!

Conversation

larryliu0820 commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented May 26, 2025

Uh oh!

facebook-github-bot commented May 26, 2025

Uh oh!

facebook-github-bot commented May 27, 2025

Uh oh!

facebook-github-bot commented May 28, 2025

Uh oh!

facebook-github-bot commented May 28, 2025

Uh oh!

facebook-github-bot commented May 28, 2025

Uh oh!

jackzhxng left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

facebook-github-bot commented May 28, 2025

Uh oh!

Uh oh!

Uh oh!

larryliu0820 commented May 26, 2025 •

edited

Loading