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

Optimize cast(uuid as varchar) #10361

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mbasmanova
Copy link
Contributor

Summary:
boost::lexical_cast used to implement cast(uuid as varchar) is very slow.

Replace with custom optimization.

Microbenchmark shows 20x improvement.

The benchmark compares uuid() with cast(uuid() as varchar). The latter includes the code of the former plus the cost of the cast. Before the change, uuid() + cast was 16s, 10s more than uuid() alone. After the change, uuid() + cast() is just 0.5s more.

Before:

============================================================================
[...]hmarks/ExpressionBenchmarkBuilder.cpp     relative  time/iter   iters/s
============================================================================
cast##no_cast                                                6.33s   157.86m
cast##as_varchar                                            16.54s    60.45m

After:

============================================================================
cast##no_cast                                                6.29s   159.10m
cast##as_varchar                                             6.81s   146.74m
----------------------------------------------------------------------------

Profile before the optimization: {F1735048022}

Differential Revision: D59229653

Summary:
boost::lexical_cast used to implement cast(uuid as varchar) is very slow.

Replace with custom optimization.

Microbenchmark shows 20x improvement.

The benchmark compares uuid() with cast(uuid() as varchar). The latter includes the code of the former plus the cost of the cast. Before the change, uuid() + cast was 16s, 10s more than uuid() alone. After the change, uuid() + cast() is just 0.5s more.

Before:

```
============================================================================
[...]hmarks/ExpressionBenchmarkBuilder.cpp     relative  time/iter   iters/s
============================================================================
cast##no_cast                                                6.33s   157.86m
cast##as_varchar                                            16.54s    60.45m
```

After:

```
============================================================================
cast##no_cast                                                6.29s   159.10m
cast##as_varchar                                             6.81s   146.74m
----------------------------------------------------------------------------
```

Profile before the optimization:  {F1735048022}

Differential Revision: D59229653
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 1, 2024
Copy link

netlify bot commented Jul 1, 2024

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 3028732
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/6682d23fef7ba90007b765d3

@facebook-github-bot
Copy link
Contributor

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

Copy link
Contributor

@xiaoxmeng xiaoxmeng left a comment

Choose a reason for hiding this comment

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

@mbasmanova Nice fix. Thanks!

Copy link
Contributor

@amitkdutta amitkdutta left a comment

Choose a reason for hiding this comment

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

Thanks @mbasmanova

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 Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants