Bug #85819 Add AArch64 optimized crc32c implementation #136
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
A similar patch has been contributed in #31, but has never appeared in any released version. |
|
Hi, thank you for your contribution. Please confirm this code is submitted under the terms of the OCA (Oracle's Contribution Agreement) you have previously signed by cutting and pasting the following text as a comment: |
ARMv8 defines PMULL crypto instruction. The new patch optimizes crc32c calculate with the instruction when available rather than original linear crc32 instructions.
|
I updated the crc32 optimization code. @akopytov ARMv8 defines PMULL crypto instruction. The new patch optimizes crc32c calculate with the PMULL instruction when available rather than original linear crc32 instructions. The result of benchmark: It shows that the performance for CRC32 of innodb on AArch64 is better than linear crc32 instruction. |
|
I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it. |
|
Hi, thank you for your contribution. Your code has been assigned to an internal queue. Please follow |
|
I test the patch in the Huawei Taishan 2208 board, but it compile failed. The error information is: error: #error You must enable AdvancedSIMD instructions to use arm_neon.h the CPU information is processor : 63 Whether lacking some or not? Thanks |
|
When running the mtr case: innodb.doublewrite, program terminated with signal SIGSEGV, Segmentation fault. |
dbussink
added a commit
to planetscale/mysql-server
that referenced
this pull request
Mar 6, 2024
The my_instr_mb function makes the incorrect assumption in multiple
places that byte lengths for the input strings can be used to short cut
certain decisions. In the case of multibyte character sets and
collations, this can't be done though since characters with a different
byte length can be considered equal under collation rules.
Take for example 'a' and 'Å' which are considered equal under the
default MySQL 8 collation utf8mb4_0900_ai_ci:
mysql> select 'a' = 'Å';
+-------------+
| 'a' = 'Å' |
+-------------+
| 1 |
+-------------+
1 row in set (0.00 sec)
The character 'a' is though encoded with 1 byte, but 'Å' is encoded with
3 bytes. When using LOCATE, it should also find the character:
mysql> select locate('a', 'Å');
+--------------------+
| locate('a', 'Å') |
+--------------------+
| 0 |
+--------------------+
1 row in set (0.00 sec)
It shows here that it's wrong, and it doesn't consider the character a
substring of the other character, which mismatches the behavior seen
when checking equality above.
This is due to a number of bugs. First of all, there is this check at
the beginning:
if (s_length <= b_length) {
To be explicit, both length variables here are byte lengths, not
character lengths of the inputs. When this check does not match and the
byte length of needle is longer than the haystack, the assumption is
made that no result can be found. This is wrong, because in case here of
a needle of 'Å' it will have 3 bytes, but the haystack of 'a' has 1
byte. We still need to search through with the proper collation
settings. Therefore the change here removes this check as it's an
incorrect optimization trying to short circuit.
There's another attempt at optimization here to reduce the maximum
string length searched:
end = b + b_length - s_length + 1;
This is also not correct because of the same reason. We can't assume
byte lengths match character lengths, so this is changed to:
end = b + b_length
to ensure that the whole haystack string is searched.
Lastly, there's another bug. The call to strnncoll again assumes that
you can truncate the haystack to the byte length. This goes wrong if the
needle is 'a' and the haystack is 'Å'. In that case, the haystack string
of 'Å' is 3 bytes long and will be cut off at 1 byte. This results in an
invalid UTF-8 input and no match on the search.
Instead, strnncoll has a prefix option, so we use that and pass in the
full string lenghts so that a proper search for a prefix is made.
The higher level Item_func_locate function also has a bug where
incorrectly byte length is used. The following is incorrect:
return start + 1;
As start here is just updated from the character position to the byte
position. Instead, we need to use start0 here to return the correct
offset.
These changes resolve the given bugs around multibyte characters and
substring searching.
Signed-off-by: Dirkjan Bussink <[email protected]>
dbussink
added a commit
to planetscale/mysql-server
that referenced
this pull request
May 21, 2024
The my_instr_mb function makes the incorrect assumption in multiple
places that byte lengths for the input strings can be used to short cut
certain decisions. In the case of multibyte character sets and
collations, this can't be done though since characters with a different
byte length can be considered equal under collation rules.
Take for example 'a' and 'Å' which are considered equal under the
default MySQL 8 collation utf8mb4_0900_ai_ci:
mysql> select 'a' = 'Å';
+-------------+
| 'a' = 'Å' |
+-------------+
| 1 |
+-------------+
1 row in set (0.00 sec)
The character 'a' is though encoded with 1 byte, but 'Å' is encoded with
3 bytes. When using LOCATE, it should also find the character:
mysql> select locate('a', 'Å');
+--------------------+
| locate('a', 'Å') |
+--------------------+
| 0 |
+--------------------+
1 row in set (0.00 sec)
It shows here that it's wrong, and it doesn't consider the character a
substring of the other character, which mismatches the behavior seen
when checking equality above.
This is due to a number of bugs. First of all, there is this check at
the beginning:
if (s_length <= b_length) {
To be explicit, both length variables here are byte lengths, not
character lengths of the inputs. When this check does not match and the
byte length of needle is longer than the haystack, the assumption is
made that no result can be found. This is wrong, because in case here of
a needle of 'Å' it will have 3 bytes, but the haystack of 'a' has 1
byte. We still need to search through with the proper collation
settings. Therefore the change here removes this check as it's an
incorrect optimization trying to short circuit.
There's another attempt at optimization here to reduce the maximum
string length searched:
end = b + b_length - s_length + 1;
This is also not correct because of the same reason. We can't assume
byte lengths match character lengths, so this is changed to:
end = b + b_length
to ensure that the whole haystack string is searched.
Lastly, there's another bug. The call to strnncoll again assumes that
you can truncate the haystack to the byte length. This goes wrong if the
needle is 'a' and the haystack is 'Å'. In that case, the haystack string
of 'Å' is 3 bytes long and will be cut off at 1 byte. This results in an
invalid UTF-8 input and no match on the search.
Instead, strnncoll has a prefix option, so we use that and pass in the
full string lenghts so that a proper search for a prefix is made.
The higher level Item_func_locate function also has a bug where
incorrectly byte length is used. The following is incorrect:
return start + 1;
As start here is just updated from the character position to the byte
position. Instead, we need to use start0 here to return the correct
offset.
These changes resolve the given bugs around multibyte characters and
substring searching.
Signed-off-by: Dirkjan Bussink <[email protected]>
dbussink
added a commit
to planetscale/mysql-server
that referenced
this pull request
May 21, 2024
The my_instr_mb function makes the incorrect assumption in multiple
places that byte lengths for the input strings can be used to short cut
certain decisions. In the case of multibyte character sets and
collations, this can't be done though since characters with a different
byte length can be considered equal under collation rules.
Take for example 'a' and 'Å' which are considered equal under the
default MySQL 8 collation utf8mb4_0900_ai_ci:
mysql> select 'a' = 'Å';
+-------------+
| 'a' = 'Å' |
+-------------+
| 1 |
+-------------+
1 row in set (0.00 sec)
The character 'a' is though encoded with 1 byte, but 'Å' is encoded with
3 bytes. When using LOCATE, it should also find the character:
mysql> select locate('a', 'Å');
+--------------------+
| locate('a', 'Å') |
+--------------------+
| 0 |
+--------------------+
1 row in set (0.00 sec)
It shows here that it's wrong, and it doesn't consider the character a
substring of the other character, which mismatches the behavior seen
when checking equality above.
This is due to a number of bugs. First of all, there is this check at
the beginning:
if (s_length <= b_length) {
To be explicit, both length variables here are byte lengths, not
character lengths of the inputs. When this check does not match and the
byte length of needle is longer than the haystack, the assumption is
made that no result can be found. This is wrong, because in case here of
a needle of 'Å' it will have 3 bytes, but the haystack of 'a' has 1
byte. We still need to search through with the proper collation
settings. Therefore the change here removes this check as it's an
incorrect optimization trying to short circuit.
There's another attempt at optimization here to reduce the maximum
string length searched:
end = b + b_length - s_length + 1;
This is also not correct because of the same reason. We can't assume
byte lengths match character lengths, so this is changed to:
end = b + b_length
to ensure that the whole haystack string is searched.
Lastly, there's another bug. The call to strnncoll again assumes that
you can truncate the haystack to the byte length. This goes wrong if the
needle is 'a' and the haystack is 'Å'. In that case, the haystack string
of 'Å' is 3 bytes long and will be cut off at 1 byte. This results in an
invalid UTF-8 input and no match on the search.
Instead, strnncoll has a prefix option, so we use that and pass in the
full string lenghts so that a proper search for a prefix is made.
The higher level Item_func_locate function also has a bug where
incorrectly byte length is used. The following is incorrect:
return start + 1;
As start here is just updated from the character position to the byte
position. Instead, we need to use start0 here to return the correct
offset.
These changes resolve the given bugs around multibyte characters and
substring searching.
Signed-off-by: Dirkjan Bussink <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ARMv8 defines a set of optional CRC32/CRC32C instructions.
The CRC32 function for AArch64 that uses these instructions will optimize the performance rather than that uses table-based lookup.
I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.
Bug #85819: https://bugs.mysql.com/bug.php?id=85819