Skip to content

ESQL: Speed up TO_IP (#126338) #126433

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

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented Apr 7, 2025

Speed up the TO_IP method by converting directly from utf-8 encoded strings to the ip encoding. Previously we did:

utf-8 -> String -> INetAddress -> ip encoding

In a step towards solving #125460 this creates three IP parsing functions, one the rejects leading zeros, one that interprets leading zeros as decimal numbers, and one the interprets leading zeros as octal numbers. IPs have historically been parsed in all three of those ways.

This plugs the "rejects leading zeros" parser into TO_IP because that's the behavior it had before.

Here is the performance:

Benchmark               Score    Error  Units
leadingZerosAreDecimal  14.007 ± 0.093  ns/op
leadingZerosAreOctal    15.020 ± 0.373  ns/op
leadingZerosRejected    14.176 ± 3.861  ns/op
original                32.950 ± 1.062  ns/op

So this is roughly 45% faster than what we had.

This includes a big chunk of #124676 - but not the behavior change - just the code that allowed it.

Speed up the TO_IP method by converting directly from utf-8 encoded
strings to the ip encoding. Previously we did:
```
utf-8 -> String -> INetAddress -> ip encoding
```

In a step towards solving elastic#125460 this creates three IP parsing
functions, one the rejects leading zeros, one that interprets leading
zeros as decimal numbers, and one the interprets leading zeros as octal
numbers. IPs have historically been parsed in all three of those ways.

This plugs the "rejects leading zeros" parser into `TO_IP` because
that's the behavior it had before.

Here is the performance:
```
Benchmark               Score    Error  Units
leadingZerosAreDecimal  14.007 ± 0.093  ns/op
leadingZerosAreOctal    15.020 ± 0.373  ns/op
leadingZerosRejected    14.176 ± 3.861  ns/op
original                32.950 ± 1.062  ns/op
```

So this is roughly 45% faster than what we had.

This includes a big chunk of elastic#124676 - but not the behavior change -
just the code that allowed it.
@nik9000 nik9000 added backport auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) v8.19.0 labels Apr 7, 2025
@nik9000 nik9000 mentioned this pull request Apr 7, 2025
@elasticsearchmachine elasticsearchmachine merged commit 366dc8c into elastic:8.x Apr 7, 2025
15 checks passed
@nik9000 nik9000 deleted the benchmark_instructions_jdk24_8x branch April 7, 2025 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) backport v8.19.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants