You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fury.apache.org by "urlyy (via GitHub)" <gi...@apache.org> on 2024/07/10 16:47:17 UTC
[PR] feat(Rust): Implement utf16 to utf8 conversion algorithm [fury]
urlyy opened a new pull request, #1730:
URL: https://github.com/apache/fury/pull/1730
## What does this PR do?
Implement an util function converting utf16 encoded string to utf8 string in Rust.
## Related issues
#1547
## Does this PR introduce any user-facing change?
- [x] Does this PR introduce any public API change?
- [ ] Does this PR introduce any binary protocol compatibility change?
## Benchmark
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [PR] feat(Rust): Implement utf16 to utf8 conversion algorithm [fury]
Posted by "theweipeng (via GitHub)" <gi...@apache.org>.
theweipeng commented on code in PR #1730:
URL: https://github.com/apache/fury/pull/1730#discussion_r1677343854
##########
rust/fury/src/util.rs:
##########
@@ -0,0 +1,90 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// Swapping the high 8 bits and the low 8 bits of a 16-bit value
+fn swap_endian(value: u16) -> u16 {
+ (value << 8) | (value >> 8)
+}
+
+pub fn to_utf8(utf16: &[u16], is_little_endian: bool) -> Result<Vec<u8>, String> {
+ // Pre-allocating approximate capacity to minimize dynamic resizing
+ let mut utf8_bytes = Vec::with_capacity(utf16.len() * 2);
+ let mut iter = utf16.iter();
+ while let Some(&wc) = iter.next() {
+ let wc = if is_little_endian {
+ swap_endian(wc)
+ } else {
+ wc
+ };
+ match wc {
+ // 1-byte UTF-8
+ // [0000|0000|0ccc|cccc] => [0ccc|cccc]
+ code_point if code_point < 0x80 => {
+ utf8_bytes.push(code_point as u8);
+ }
+ // 2-byte UTF-8
+ code_point if code_point < 0x800 => {
+ // [0000|0bbb|bbcc|cccc] => [110|bbbbb], [10|cccccc]
+ let second = ((code_point & 0b11_1111) as u8) | 0b1000_0000;
+ let first = ((code_point >> 6 & 0b1_1111) as u8) | 0b1100_0000;
+ utf8_bytes.push(first);
+ utf8_bytes.push(second);
+ }
+ // Surrogate pair (4-byte UTF-8)
+ wc1 if (0xd800..=0xdbff).contains(&wc1) => {
+ // Need extra byte
+ if let Some(&wc2) = iter.next() {
+ let wc2 = if is_little_endian {
+ swap_endian(wc2)
+ } else {
+ wc2
+ };
+ if !(0xdc00..=0xdfff).contains(&wc2) {
+ return Err("Invalid UTF-16 string: wrong surrogate pair".to_string());
+ }
+ // utf16 to unicode
+ let code_point =
+ ((((wc1 as u32) - 0xd800) << 10) | ((wc2 as u32) - 0xdc00)) + 0x10000;
+ // 11110??? 10?????? 10?????? 10??????
+ // Need 21 bit suffix of code_point
+ let fourth = ((code_point & 0b11_1111) as u8) | 0b1000_0000;
+ let third = ((code_point >> 6 & 0b11_1111) as u8) | 0b1000_0000;
+ let second = ((code_point >> 12 & 0b11_1111) as u8) | 0b1000_0000;
+ let first = ((code_point >> 18 & 0b111) as u8) | 0b1111_0000;
+ utf8_bytes.push(first);
Review Comment:
You can cast u8 ptr to u32 ptr. for instance `buf.as_mut_ptr() as *mut i32;`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [PR] feat(Rust): Implement utf16 to utf8 conversion algorithm [fury]
Posted by "theweipeng (via GitHub)" <gi...@apache.org>.
theweipeng commented on code in PR #1730:
URL: https://github.com/apache/fury/pull/1730#discussion_r1681231212
##########
rust/fury/src/util.rs:
##########
@@ -0,0 +1,122 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// Swapping the high 8 bits and the low 8 bits of a 16-bit value
+fn swap_endian(value: u16) -> u16 {
+ (value << 8) | (value >> 8)
+}
+
+pub fn to_utf8(utf16: &[u16], is_little_endian: bool) -> Result<Vec<u8>, String> {
+ // Pre-allocating capacity to avoid dynamic resizing
+ // Longest case: 1 u16 to 3 u8
+ let mut utf8_bytes: Vec<u8> = Vec::with_capacity(utf16.len() * 3);
+ // For unsafe write to Vec
+ let ptr = utf8_bytes.as_mut_ptr();
+ let mut offset = 0;
+ let mut iter = utf16.iter();
+ while let Some(&wc) = iter.next() {
+ // Using big endian in this conversion
+ let wc = if is_little_endian {
+ swap_endian(wc)
+ } else {
+ wc
+ };
+ match wc {
+ code_point if code_point < 0x80 => {
+ // 1-byte UTF-8
+ // [0000|0000|0ccc|cccc] => [0ccc|cccc]
+ unsafe {
+ ptr.add(offset).write(code_point as u8);
+ }
+ offset += 1;
+ }
+ code_point if code_point < 0x800 => {
+ // 2-byte UTF-8
+ // [0000|0bbb|bbcc|cccc] => [110|bbbbb], [10|cccccc]
+ // write a u16 to [u8;2] is affected by the processor's endianness.
+ #[cfg(target_endian = "little")]
Review Comment:
We can use copy_from_slice
```rust
let sl = utf8_bytes.as_mut_slice()
sl.copy_from_slice(&[u1,u2,u3,u4]);
```
Or copy_nonoverlapping
```rust
ptr::copy_nonoverlapping(src.as_ptr(), self.as_mut_ptr(), self.len());
```
to avoid the multiple times memory write overhead. Meanwhile, enhance the readability of the code
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [PR] feat(Rust): Implement utf16 to utf8 conversion algorithm [fury]
Posted by "theweipeng (via GitHub)" <gi...@apache.org>.
theweipeng commented on code in PR #1730:
URL: https://github.com/apache/fury/pull/1730#discussion_r1677261008
##########
rust/fury/src/util.rs:
##########
@@ -0,0 +1,90 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// Swapping the high 8 bits and the low 8 bits of a 16-bit value
+fn swap_endian(value: u16) -> u16 {
+ (value << 8) | (value >> 8)
+}
+
+pub fn to_utf8(utf16: &[u16], is_little_endian: bool) -> Result<Vec<u8>, String> {
+ // Pre-allocating approximate capacity to minimize dynamic resizing
+ let mut utf8_bytes = Vec::with_capacity(utf16.len() * 2);
Review Comment:
A UTF16 char can be encoded into up to 4 UTF8 bytes. Perhaps we can change it to `Vec::with_capacity(utf16.len() * 4);`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [PR] feat(Rust): Implement utf16 to utf8 conversion algorithm [fury]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #1730:
URL: https://github.com/apache/fury/pull/1730#discussion_r1674093742
##########
rust/fury/src/util.rs:
##########
@@ -0,0 +1,93 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#[derive(thiserror::Error, Debug)]
+pub enum UtilError {
Review Comment:
Could we use standard error instead? UtilError looks vague to me
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [PR] feat(Rust): Implement utf16 to utf8 conversion algorithm [fury]
Posted by "urlyy (via GitHub)" <gi...@apache.org>.
urlyy commented on code in PR #1730:
URL: https://github.com/apache/fury/pull/1730#discussion_r1677331465
##########
rust/fury/src/util.rs:
##########
@@ -0,0 +1,90 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// Swapping the high 8 bits and the low 8 bits of a 16-bit value
+fn swap_endian(value: u16) -> u16 {
+ (value << 8) | (value >> 8)
+}
+
+pub fn to_utf8(utf16: &[u16], is_little_endian: bool) -> Result<Vec<u8>, String> {
+ // Pre-allocating approximate capacity to minimize dynamic resizing
+ let mut utf8_bytes = Vec::with_capacity(utf16.len() * 2);
+ let mut iter = utf16.iter();
+ while let Some(&wc) = iter.next() {
+ let wc = if is_little_endian {
+ swap_endian(wc)
+ } else {
+ wc
+ };
+ match wc {
+ // 1-byte UTF-8
+ // [0000|0000|0ccc|cccc] => [0ccc|cccc]
+ code_point if code_point < 0x80 => {
+ utf8_bytes.push(code_point as u8);
+ }
+ // 2-byte UTF-8
+ code_point if code_point < 0x800 => {
+ // [0000|0bbb|bbcc|cccc] => [110|bbbbb], [10|cccccc]
+ let second = ((code_point & 0b11_1111) as u8) | 0b1000_0000;
+ let first = ((code_point >> 6 & 0b1_1111) as u8) | 0b1100_0000;
+ utf8_bytes.push(first);
+ utf8_bytes.push(second);
+ }
+ // Surrogate pair (4-byte UTF-8)
+ wc1 if (0xd800..=0xdbff).contains(&wc1) => {
+ // Need extra byte
+ if let Some(&wc2) = iter.next() {
+ let wc2 = if is_little_endian {
+ swap_endian(wc2)
+ } else {
+ wc2
+ };
+ if !(0xdc00..=0xdfff).contains(&wc2) {
+ return Err("Invalid UTF-16 string: wrong surrogate pair".to_string());
+ }
+ // utf16 to unicode
+ let code_point =
+ ((((wc1 as u32) - 0xd800) << 10) | ((wc2 as u32) - 0xdc00)) + 0x10000;
+ // 11110??? 10?????? 10?????? 10??????
+ // Need 21 bit suffix of code_point
+ let fourth = ((code_point & 0b11_1111) as u8) | 0b1000_0000;
+ let third = ((code_point >> 6 & 0b11_1111) as u8) | 0b1000_0000;
+ let second = ((code_point >> 12 & 0b11_1111) as u8) | 0b1000_0000;
+ let first = ((code_point >> 18 & 0b111) as u8) | 0b1111_0000;
+ utf8_bytes.push(first);
Review Comment:
Thanks for your advice. I have replaced `push` with `ptr.add(offset).write(value)` inside an `unsafe` block. However, I can't write a `u32` into `Vec<u8>` at once, it causes error `mismatched types:
expected u8, found u32`. So I'm still using multiple write operations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [PR] feat(Rust): Implement utf16 to utf8 conversion algorithm [fury]
Posted by "urlyy (via GitHub)" <gi...@apache.org>.
urlyy commented on code in PR #1730:
URL: https://github.com/apache/fury/pull/1730#discussion_r1677331465
##########
rust/fury/src/util.rs:
##########
@@ -0,0 +1,90 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// Swapping the high 8 bits and the low 8 bits of a 16-bit value
+fn swap_endian(value: u16) -> u16 {
+ (value << 8) | (value >> 8)
+}
+
+pub fn to_utf8(utf16: &[u16], is_little_endian: bool) -> Result<Vec<u8>, String> {
+ // Pre-allocating approximate capacity to minimize dynamic resizing
+ let mut utf8_bytes = Vec::with_capacity(utf16.len() * 2);
+ let mut iter = utf16.iter();
+ while let Some(&wc) = iter.next() {
+ let wc = if is_little_endian {
+ swap_endian(wc)
+ } else {
+ wc
+ };
+ match wc {
+ // 1-byte UTF-8
+ // [0000|0000|0ccc|cccc] => [0ccc|cccc]
+ code_point if code_point < 0x80 => {
+ utf8_bytes.push(code_point as u8);
+ }
+ // 2-byte UTF-8
+ code_point if code_point < 0x800 => {
+ // [0000|0bbb|bbcc|cccc] => [110|bbbbb], [10|cccccc]
+ let second = ((code_point & 0b11_1111) as u8) | 0b1000_0000;
+ let first = ((code_point >> 6 & 0b1_1111) as u8) | 0b1100_0000;
+ utf8_bytes.push(first);
+ utf8_bytes.push(second);
+ }
+ // Surrogate pair (4-byte UTF-8)
+ wc1 if (0xd800..=0xdbff).contains(&wc1) => {
+ // Need extra byte
+ if let Some(&wc2) = iter.next() {
+ let wc2 = if is_little_endian {
+ swap_endian(wc2)
+ } else {
+ wc2
+ };
+ if !(0xdc00..=0xdfff).contains(&wc2) {
+ return Err("Invalid UTF-16 string: wrong surrogate pair".to_string());
+ }
+ // utf16 to unicode
+ let code_point =
+ ((((wc1 as u32) - 0xd800) << 10) | ((wc2 as u32) - 0xdc00)) + 0x10000;
+ // 11110??? 10?????? 10?????? 10??????
+ // Need 21 bit suffix of code_point
+ let fourth = ((code_point & 0b11_1111) as u8) | 0b1000_0000;
+ let third = ((code_point >> 6 & 0b11_1111) as u8) | 0b1000_0000;
+ let second = ((code_point >> 12 & 0b11_1111) as u8) | 0b1000_0000;
+ let first = ((code_point >> 18 & 0b111) as u8) | 0b1111_0000;
+ utf8_bytes.push(first);
Review Comment:
Thanks for your advice. I have replaced `push` with `ptr.add(offset).write(value)` inside an `unsafe` block. However, I can't write a `u32` into `Vec<u8>` at once, so I'm still using multiple write operations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [PR] feat(Rust): Implement utf16 to utf8 conversion algorithm [fury]
Posted by "urlyy (via GitHub)" <gi...@apache.org>.
urlyy commented on PR #1730:
URL: https://github.com/apache/fury/pull/1730#issuecomment-2227748860
I also find a dataset of character set: https://github.com/lemire/unicode_lipsum, hope this will be useful for future benchmarks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [PR] feat(Rust): Implement utf16 to utf8 conversion algorithm [fury]
Posted by "theweipeng (via GitHub)" <gi...@apache.org>.
theweipeng merged PR #1730:
URL: https://github.com/apache/fury/pull/1730
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [PR] feat(Rust): Implement utf16 to utf8 conversion algorithm [fury]
Posted by "theweipeng (via GitHub)" <gi...@apache.org>.
theweipeng commented on code in PR #1730:
URL: https://github.com/apache/fury/pull/1730#discussion_r1677261008
##########
rust/fury/src/util.rs:
##########
@@ -0,0 +1,90 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// Swapping the high 8 bits and the low 8 bits of a 16-bit value
+fn swap_endian(value: u16) -> u16 {
+ (value << 8) | (value >> 8)
+}
+
+pub fn to_utf8(utf16: &[u16], is_little_endian: bool) -> Result<Vec<u8>, String> {
+ // Pre-allocating approximate capacity to minimize dynamic resizing
+ let mut utf8_bytes = Vec::with_capacity(utf16.len() * 2);
Review Comment:
One UTF16 char can be encoded into up to 4 UTF8 bytes. Perhaps we can change it to `Vec::with_capacity(utf16.len() * 4);`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [PR] feat(Rust): Implement utf16 to utf8 conversion algorithm [fury]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on PR #1730:
URL: https://github.com/apache/fury/pull/1730#issuecomment-2223091667
Looks good to me overall, only some minor error handle issues. @theweipeng do you have some suggestion?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [PR] feat(Rust): Implement utf16 to utf8 conversion algorithm [fury]
Posted by "urlyy (via GitHub)" <gi...@apache.org>.
urlyy commented on code in PR #1730:
URL: https://github.com/apache/fury/pull/1730#discussion_r1677321043
##########
rust/fury/src/util.rs:
##########
@@ -0,0 +1,90 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// Swapping the high 8 bits and the low 8 bits of a 16-bit value
+fn swap_endian(value: u16) -> u16 {
+ (value << 8) | (value >> 8)
+}
+
+pub fn to_utf8(utf16: &[u16], is_little_endian: bool) -> Result<Vec<u8>, String> {
+ // Pre-allocating approximate capacity to minimize dynamic resizing
+ let mut utf8_bytes = Vec::with_capacity(utf16.len() * 2);
Review Comment:
I think the maximum length should be `utf16.len()*3`, because `four u8` uses `two u16`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [PR] feat(Rust): Implement utf16 to utf8 conversion algorithm [fury]
Posted by "urlyy (via GitHub)" <gi...@apache.org>.
urlyy commented on code in PR #1730:
URL: https://github.com/apache/fury/pull/1730#discussion_r1678109650
##########
rust/fury/src/util.rs:
##########
@@ -0,0 +1,90 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// Swapping the high 8 bits and the low 8 bits of a 16-bit value
+fn swap_endian(value: u16) -> u16 {
+ (value << 8) | (value >> 8)
+}
+
+pub fn to_utf8(utf16: &[u16], is_little_endian: bool) -> Result<Vec<u8>, String> {
+ // Pre-allocating approximate capacity to minimize dynamic resizing
+ let mut utf8_bytes = Vec::with_capacity(utf16.len() * 2);
+ let mut iter = utf16.iter();
+ while let Some(&wc) = iter.next() {
+ let wc = if is_little_endian {
+ swap_endian(wc)
+ } else {
+ wc
+ };
+ match wc {
+ // 1-byte UTF-8
+ // [0000|0000|0ccc|cccc] => [0ccc|cccc]
+ code_point if code_point < 0x80 => {
+ utf8_bytes.push(code_point as u8);
+ }
+ // 2-byte UTF-8
+ code_point if code_point < 0x800 => {
+ // [0000|0bbb|bbcc|cccc] => [110|bbbbb], [10|cccccc]
+ let second = ((code_point & 0b11_1111) as u8) | 0b1000_0000;
+ let first = ((code_point >> 6 & 0b1_1111) as u8) | 0b1100_0000;
+ utf8_bytes.push(first);
+ utf8_bytes.push(second);
+ }
+ // Surrogate pair (4-byte UTF-8)
+ wc1 if (0xd800..=0xdbff).contains(&wc1) => {
+ // Need extra byte
+ if let Some(&wc2) = iter.next() {
+ let wc2 = if is_little_endian {
+ swap_endian(wc2)
+ } else {
+ wc2
+ };
+ if !(0xdc00..=0xdfff).contains(&wc2) {
+ return Err("Invalid UTF-16 string: wrong surrogate pair".to_string());
+ }
+ // utf16 to unicode
+ let code_point =
+ ((((wc1 as u32) - 0xd800) << 10) | ((wc2 as u32) - 0xdc00)) + 0x10000;
+ // 11110??? 10?????? 10?????? 10??????
+ // Need 21 bit suffix of code_point
+ let fourth = ((code_point & 0b11_1111) as u8) | 0b1000_0000;
+ let third = ((code_point >> 6 & 0b11_1111) as u8) | 0b1000_0000;
+ let second = ((code_point >> 12 & 0b11_1111) as u8) | 0b1000_0000;
+ let first = ((code_point >> 18 & 0b111) as u8) | 0b1111_0000;
+ utf8_bytes.push(first);
Review Comment:
I found that write u32 to [u8:4] will be affected by processor's endianness.
```rust
fn main() {
let mut vec = vec![0u8; 4];
unsafe {
let ptr = vec.as_mut_ptr() as *mut u32;
ptr.write(0xa1b2c3d4);
vec.set_len(4);
}
for i in 0..vec.len() {
print!("0x{:02x} ", vec[i]);
}
// print: 0xd4 0xc3 0xb2 0xa1
}
````
So I use `#[cfg(target_endian = "little | big")]` to handle this.
```rust
#[cfg(target_endian = "little")]
ptr.write(0xd4c3b2a1);
#[cfg(target_endian = "big")]
ptr.write(0xa1b2c3d4);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [PR] feat(Rust): Implement utf16 to utf8 conversion algorithm [fury]
Posted by "theweipeng (via GitHub)" <gi...@apache.org>.
theweipeng commented on code in PR #1730:
URL: https://github.com/apache/fury/pull/1730#discussion_r1677263494
##########
rust/fury/src/util.rs:
##########
@@ -0,0 +1,90 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// Swapping the high 8 bits and the low 8 bits of a 16-bit value
+fn swap_endian(value: u16) -> u16 {
+ (value << 8) | (value >> 8)
+}
+
+pub fn to_utf8(utf16: &[u16], is_little_endian: bool) -> Result<Vec<u8>, String> {
+ // Pre-allocating approximate capacity to minimize dynamic resizing
+ let mut utf8_bytes = Vec::with_capacity(utf16.len() * 2);
+ let mut iter = utf16.iter();
+ while let Some(&wc) = iter.next() {
+ let wc = if is_little_endian {
+ swap_endian(wc)
+ } else {
+ wc
+ };
+ match wc {
+ // 1-byte UTF-8
+ // [0000|0000|0ccc|cccc] => [0ccc|cccc]
+ code_point if code_point < 0x80 => {
+ utf8_bytes.push(code_point as u8);
+ }
+ // 2-byte UTF-8
+ code_point if code_point < 0x800 => {
+ // [0000|0bbb|bbcc|cccc] => [110|bbbbb], [10|cccccc]
+ let second = ((code_point & 0b11_1111) as u8) | 0b1000_0000;
+ let first = ((code_point >> 6 & 0b1_1111) as u8) | 0b1100_0000;
+ utf8_bytes.push(first);
+ utf8_bytes.push(second);
+ }
+ // Surrogate pair (4-byte UTF-8)
+ wc1 if (0xd800..=0xdbff).contains(&wc1) => {
+ // Need extra byte
+ if let Some(&wc2) = iter.next() {
+ let wc2 = if is_little_endian {
+ swap_endian(wc2)
+ } else {
+ wc2
+ };
+ if !(0xdc00..=0xdfff).contains(&wc2) {
+ return Err("Invalid UTF-16 string: wrong surrogate pair".to_string());
+ }
+ // utf16 to unicode
+ let code_point =
+ ((((wc1 as u32) - 0xd800) << 10) | ((wc2 as u32) - 0xdc00)) + 0x10000;
+ // 11110??? 10?????? 10?????? 10??????
+ // Need 21 bit suffix of code_point
+ let fourth = ((code_point & 0b11_1111) as u8) | 0b1000_0000;
+ let third = ((code_point >> 6 & 0b11_1111) as u8) | 0b1000_0000;
+ let second = ((code_point >> 12 & 0b11_1111) as u8) | 0b1000_0000;
+ let first = ((code_point >> 18 & 0b111) as u8) | 0b1111_0000;
+ utf8_bytes.push(first);
Review Comment:
`push` will check the memory boundary, since we initialize utf8_bytes with a fixed capacity, we can avoid this overhead by using a more efficient way. Merging four u8 values into a single u32 to reduce additional memory writes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [PR] feat(Rust): Implement utf16 to utf8 conversion algorithm [fury]
Posted by "theweipeng (via GitHub)" <gi...@apache.org>.
theweipeng commented on code in PR #1730:
URL: https://github.com/apache/fury/pull/1730#discussion_r1681231212
##########
rust/fury/src/util.rs:
##########
@@ -0,0 +1,122 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// Swapping the high 8 bits and the low 8 bits of a 16-bit value
+fn swap_endian(value: u16) -> u16 {
+ (value << 8) | (value >> 8)
+}
+
+pub fn to_utf8(utf16: &[u16], is_little_endian: bool) -> Result<Vec<u8>, String> {
+ // Pre-allocating capacity to avoid dynamic resizing
+ // Longest case: 1 u16 to 3 u8
+ let mut utf8_bytes: Vec<u8> = Vec::with_capacity(utf16.len() * 3);
+ // For unsafe write to Vec
+ let ptr = utf8_bytes.as_mut_ptr();
+ let mut offset = 0;
+ let mut iter = utf16.iter();
+ while let Some(&wc) = iter.next() {
+ // Using big endian in this conversion
+ let wc = if is_little_endian {
+ swap_endian(wc)
+ } else {
+ wc
+ };
+ match wc {
+ code_point if code_point < 0x80 => {
+ // 1-byte UTF-8
+ // [0000|0000|0ccc|cccc] => [0ccc|cccc]
+ unsafe {
+ ptr.add(offset).write(code_point as u8);
+ }
+ offset += 1;
+ }
+ code_point if code_point < 0x800 => {
+ // 2-byte UTF-8
+ // [0000|0bbb|bbcc|cccc] => [110|bbbbb], [10|cccccc]
+ // write a u16 to [u8;2] is affected by the processor's endianness.
+ #[cfg(target_endian = "little")]
Review Comment:
We can use copy_from_slice
```
let sl = utf8_bytes.as_mut_slice()
sl.copy_from_slice(&[u1,u2,u3,u4]);
```
Or copy_nonoverlapping
```
ptr::copy_nonoverlapping(src.as_ptr(), self.as_mut_ptr(), self.len());
```
```
to avoid the multiple times memory write overhead. Meanwhile, enhance the readability of the code
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [PR] feat(Rust): Implement utf16 to utf8 conversion algorithm [fury]
Posted by "urlyy (via GitHub)" <gi...@apache.org>.
urlyy commented on code in PR #1730:
URL: https://github.com/apache/fury/pull/1730#discussion_r1678109650
##########
rust/fury/src/util.rs:
##########
@@ -0,0 +1,90 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// Swapping the high 8 bits and the low 8 bits of a 16-bit value
+fn swap_endian(value: u16) -> u16 {
+ (value << 8) | (value >> 8)
+}
+
+pub fn to_utf8(utf16: &[u16], is_little_endian: bool) -> Result<Vec<u8>, String> {
+ // Pre-allocating approximate capacity to minimize dynamic resizing
+ let mut utf8_bytes = Vec::with_capacity(utf16.len() * 2);
+ let mut iter = utf16.iter();
+ while let Some(&wc) = iter.next() {
+ let wc = if is_little_endian {
+ swap_endian(wc)
+ } else {
+ wc
+ };
+ match wc {
+ // 1-byte UTF-8
+ // [0000|0000|0ccc|cccc] => [0ccc|cccc]
+ code_point if code_point < 0x80 => {
+ utf8_bytes.push(code_point as u8);
+ }
+ // 2-byte UTF-8
+ code_point if code_point < 0x800 => {
+ // [0000|0bbb|bbcc|cccc] => [110|bbbbb], [10|cccccc]
+ let second = ((code_point & 0b11_1111) as u8) | 0b1000_0000;
+ let first = ((code_point >> 6 & 0b1_1111) as u8) | 0b1100_0000;
+ utf8_bytes.push(first);
+ utf8_bytes.push(second);
+ }
+ // Surrogate pair (4-byte UTF-8)
+ wc1 if (0xd800..=0xdbff).contains(&wc1) => {
+ // Need extra byte
+ if let Some(&wc2) = iter.next() {
+ let wc2 = if is_little_endian {
+ swap_endian(wc2)
+ } else {
+ wc2
+ };
+ if !(0xdc00..=0xdfff).contains(&wc2) {
+ return Err("Invalid UTF-16 string: wrong surrogate pair".to_string());
+ }
+ // utf16 to unicode
+ let code_point =
+ ((((wc1 as u32) - 0xd800) << 10) | ((wc2 as u32) - 0xdc00)) + 0x10000;
+ // 11110??? 10?????? 10?????? 10??????
+ // Need 21 bit suffix of code_point
+ let fourth = ((code_point & 0b11_1111) as u8) | 0b1000_0000;
+ let third = ((code_point >> 6 & 0b11_1111) as u8) | 0b1000_0000;
+ let second = ((code_point >> 12 & 0b11_1111) as u8) | 0b1000_0000;
+ let first = ((code_point >> 18 & 0b111) as u8) | 0b1111_0000;
+ utf8_bytes.push(first);
Review Comment:
I found that write u32 to [u8:4] would be affected by processor's endianness.
```rust
fn main() {
let mut vec = vec![0u8; 4];
unsafe {
let ptr = vec.as_mut_ptr() as *mut u32;
ptr.write(0xa1b2c3d4);
vec.set_len(4);
}
for i in 0..vec.len() {
print!("0x{:02x} ", vec[i]);
}
// print: 0xd4 0xc3 0xb2 0xa1
}
````
So I use `#[cfg(target_endian = "little | big")]` to handle this.
```rust
#[cfg(target_endian = "little")]
ptr.write(0xd4c3b2a1);
#[cfg(target_endian = "big")]
ptr.write(0xa1b2c3d4);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org