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

enhance string util functions #21893

Merged
merged 2 commits into from
Aug 29, 2024
Merged

enhance string util functions #21893

merged 2 commits into from
Aug 29, 2024

Conversation

fs-eire
Copy link
Contributor

@fs-eire fs-eire commented Aug 28, 2024

Description

  • make MakeString force inline
  • refactor ORT_FORCEINLINE macro - move to one place to avoid macro redefinition error
  • add a StringJoin utility

Motivation and Context

snnn
snnn previously approved these changes Aug 28, 2024
@pranavsharma
Copy link
Contributor

What's the need for writing a wrapper for ostringstream? ostringstream is already general enough.

@fs-eire
Copy link
Contributor Author

fs-eire commented Aug 28, 2024

What's the need for writing a wrapper for ostringstream? ostringstream is already general enough.

Using a function call for appending to ostream is helpful when using with macros or other template functions.

MakeString is already used in macros like ORT_ENFORCE(). StringJoin is also going to be useful in WebGPU EP that I am currently working on.

@fs-eire fs-eire merged commit 32af2ba into main Aug 29, 2024
96 of 97 checks passed
@fs-eire fs-eire deleted the fs-eire/string branch August 29, 2024 17:37
@fs-eire fs-eire mentioned this pull request Aug 29, 2024
6 tasks
fs-eire added a commit that referenced this pull request Aug 31, 2024
fs-eire added a commit that referenced this pull request Aug 31, 2024
fs-eire added a commit that referenced this pull request Sep 3, 2024
### Description

revert forceinline for MakeString.

This change reverts #21893.
The forceinline was introduced for performance considerations, however
it turns out to have some notable binary size increase, which is a
concern for some binary size sensitive platforms like Android.

I made a few tests locally and found it is not related to whether or not
have used the template struct `if_char_array_make_ptr_t` trick. So I
have to revert this back.
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.

4 participants