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

reduce has_authority check overhead #741

Closed
wants to merge 1 commit into from
Closed

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Sep 27, 2024

We are never setting has_authority to false. I am not sure if we should or not. Let's merge this once we are sure.

Before

BasicBench_AdaURL_url                206796 ns       206701 ns         3421 GHz=4.59137 cycle/byte=44.7268 cycles/url=1.08058k instructions/byte=110.33 instructions/cycle=2.46674 instructions/ns=11.3257 instructions/url=2.66552k ns/url=235.351 speed=99.5835M/s time/byte=10.0418ns time/url=242.607ns url/s=4.1219M/s
BasicBench_AdaURL_url_aggregator     177953 ns       177597 ns         4035 GHz=4.49142 cycle/byte=36.2755 cycles/url=876.401 instructions/byte=113.032 instructions/cycle=3.11593 instructions/ns=13.995 instructions/url=2.73081k ns/url=195.128 speed=115.903M/s time/byte=8.62789ns time/url=208.447ns url/s=4.79739M/s

After

BasicBench_AdaURL_url                200141 ns       199923 ns         3485 GHz=4.5919 cycle/byte=42.8454 cycles/url=1.03513k instructions/byte=110.222 instructions/cycle=2.57254 instructions/ns=11.8129 instructions/url=2.66291k ns/url=225.425 speed=102.96M/s time/byte=9.71253ns time/url=234.651ns url/s=4.26165M/s
BasicBench_AdaURL_url_aggregator     175378 ns       175344 ns         3969 GHz=4.59219 cycle/byte=37.1822 cycles/url=898.308 instructions/byte=112.236 instructions/cycle=3.01854 instructions/ns=13.8617 instructions/url=2.71158k ns/url=195.616 speed=117.392M/s time/byte=8.51846ns time/url=205.803ns url/s=4.85902M/s

@anonrig anonrig requested a review from lemire September 27, 2024 20:10
Copy link
Member

@lemire lemire left a comment

Choose a reason for hiding this comment

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

Intriguing optimization.

The compiler might be smart enough to figure it out, though maybe not.

I am not sure why your BasicBench_AdaURL_url results have changed. :-/

@lemire
Copy link
Member

lemire commented Sep 27, 2024

@anonrig

It seems to break the tests.

Copy link
Member

@lemire lemire left a comment

Choose a reason for hiding this comment

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

Tests are failing.

@anonrig anonrig closed this Nov 14, 2024
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.

2 participants