github-actions[bot] commented on code in PR #35713:
URL: https://github.com/apache/doris/pull/35713#discussion_r1621843550
##########
be/src/gutil/strings/fastmem.h:
##########
@@ -94,65 +99,113 @@ inline int fastmemcmp_inlined(const void* a_void, const
void* b_void, size_t n)
return 0;
}
-// The standard memcpy operation is slow for variable small sizes.
-// This implementation inlines the optimal realization for sizes 1 to 16.
-// To avoid code bloat don't use it in case of not performance-critical spots,
-// nor when you don't expect very frequent values of size <= 16.
-inline void memcpy_inlined(void* dst, const void* src, size_t size) {
- // Compiler inlines code with minimal amount of data movement when third
- // parameter of memcpy is a constant.
- switch (size) {
- case 1:
- memcpy(dst, src, 1);
- break;
- case 2:
- memcpy(dst, src, 2);
- break;
- case 3:
- memcpy(dst, src, 3);
- break;
- case 4:
- memcpy(dst, src, 4);
- break;
- case 5:
- memcpy(dst, src, 5);
- break;
- case 6:
- memcpy(dst, src, 6);
- break;
- case 7:
- memcpy(dst, src, 7);
- break;
- case 8:
- memcpy(dst, src, 8);
- break;
- case 9:
- memcpy(dst, src, 9);
- break;
- case 10:
- memcpy(dst, src, 10);
- break;
- case 11:
- memcpy(dst, src, 11);
- break;
- case 12:
- memcpy(dst, src, 12);
- break;
- case 13:
- memcpy(dst, src, 13);
- break;
- case 14:
- memcpy(dst, src, 14);
- break;
- case 15:
- memcpy(dst, src, 15);
- break;
- case 16:
- memcpy(dst, src, 16);
- break;
- default:
- memcpy(dst, src, size);
- break;
+ALWAYS_INLINE inline void memcpy_inlined(void* __restrict _dst, const void*
__restrict _src,
Review Comment:
warning: function 'memcpy_inlined' exceeds recommended size/complexity
thresholds [readability-function-size]
```cpp
ALWAYS_INLINE inline void memcpy_inlined(void* __restrict _dst, const void*
__restrict _src,
^
```
<details>
<summary>Additional context</summary>
**be/src/gutil/strings/fastmem.h:101:** 107 lines including whitespace and
comments (threshold 80)
```cpp
ALWAYS_INLINE inline void memcpy_inlined(void* __restrict _dst, const void*
__restrict _src,
^
```
</details>
##########
be/src/gutil/strings/fastmem.h:
##########
@@ -94,65 +99,113 @@
return 0;
}
-// The standard memcpy operation is slow for variable small sizes.
-// This implementation inlines the optimal realization for sizes 1 to 16.
-// To avoid code bloat don't use it in case of not performance-critical spots,
-// nor when you don't expect very frequent values of size <= 16.
-inline void memcpy_inlined(void* dst, const void* src, size_t size) {
- // Compiler inlines code with minimal amount of data movement when third
- // parameter of memcpy is a constant.
- switch (size) {
- case 1:
- memcpy(dst, src, 1);
- break;
- case 2:
- memcpy(dst, src, 2);
- break;
- case 3:
- memcpy(dst, src, 3);
- break;
- case 4:
- memcpy(dst, src, 4);
- break;
- case 5:
- memcpy(dst, src, 5);
- break;
- case 6:
- memcpy(dst, src, 6);
- break;
- case 7:
- memcpy(dst, src, 7);
- break;
- case 8:
- memcpy(dst, src, 8);
- break;
- case 9:
- memcpy(dst, src, 9);
- break;
- case 10:
- memcpy(dst, src, 10);
- break;
- case 11:
- memcpy(dst, src, 11);
- break;
- case 12:
- memcpy(dst, src, 12);
- break;
- case 13:
- memcpy(dst, src, 13);
- break;
- case 14:
- memcpy(dst, src, 14);
- break;
- case 15:
- memcpy(dst, src, 15);
- break;
- case 16:
- memcpy(dst, src, 16);
- break;
- default:
- memcpy(dst, src, size);
- break;
+ALWAYS_INLINE inline void memcpy_inlined(void* __restrict _dst, const void*
__restrict _src,
+ size_t size) {
+ auto dst = static_cast<uint8_t*>(_dst);
Review Comment:
warning: 'auto dst' can be declared as 'auto *dst'
[readability-qualified-auto]
```suggestion
auto *dst = static_cast<uint8_t*>(_dst);
```
##########
be/src/gutil/strings/fastmem.h:
##########
@@ -94,65 +99,113 @@
return 0;
}
-// The standard memcpy operation is slow for variable small sizes.
-// This implementation inlines the optimal realization for sizes 1 to 16.
-// To avoid code bloat don't use it in case of not performance-critical spots,
-// nor when you don't expect very frequent values of size <= 16.
-inline void memcpy_inlined(void* dst, const void* src, size_t size) {
- // Compiler inlines code with minimal amount of data movement when third
- // parameter of memcpy is a constant.
- switch (size) {
- case 1:
- memcpy(dst, src, 1);
- break;
- case 2:
- memcpy(dst, src, 2);
- break;
- case 3:
- memcpy(dst, src, 3);
- break;
- case 4:
- memcpy(dst, src, 4);
- break;
- case 5:
- memcpy(dst, src, 5);
- break;
- case 6:
- memcpy(dst, src, 6);
- break;
- case 7:
- memcpy(dst, src, 7);
- break;
- case 8:
- memcpy(dst, src, 8);
- break;
- case 9:
- memcpy(dst, src, 9);
- break;
- case 10:
- memcpy(dst, src, 10);
- break;
- case 11:
- memcpy(dst, src, 11);
- break;
- case 12:
- memcpy(dst, src, 12);
- break;
- case 13:
- memcpy(dst, src, 13);
- break;
- case 14:
- memcpy(dst, src, 14);
- break;
- case 15:
- memcpy(dst, src, 15);
- break;
- case 16:
- memcpy(dst, src, 16);
- break;
- default:
- memcpy(dst, src, size);
- break;
+ALWAYS_INLINE inline void memcpy_inlined(void* __restrict _dst, const void*
__restrict _src,
+ size_t size) {
+ auto dst = static_cast<uint8_t*>(_dst);
+ auto src = static_cast<const uint8_t*>(_src);
Review Comment:
warning: 'auto src' can be declared as 'const auto *src'
[readability-qualified-auto]
```suggestion
const auto *src = static_cast<const uint8_t*>(_src);
```
--
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-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
