Skip to content

Conversation

@maximyurchuk
Copy link
Collaborator

@maximyurchuk maximyurchuk commented Jul 3, 2024

Changelog entry

  • remove unused args
  • remove unused funtions

Changelog category

  • Not for changelog (changelog entry is not required)

Additional information

@github-actions
Copy link

github-actions bot commented Jul 3, 2024

2024-07-03 10:33:07 UTC Pre-commit check for 96bdac2 has started.
2024-07-03 10:36:35 UTC Build linux-x86_64-release-clang14 is running...
🔴 2024-07-03 11:08:05 UTC Build failed. see the build logs.

@github-actions
Copy link

github-actions bot commented Jul 3, 2024

2024-07-03 10:52:08 UTC Pre-commit check for 96bdac2 has started.
2024-07-03 10:55:34 UTC Build linux-x86_64-relwithdebinfo is running...
🔴 2024-07-03 11:20:24 UTC Build failed. see the build logs.
🔴 2024-07-03 11:20:25 UTC Tests run skipped.

@github-actions
Copy link

github-actions bot commented Jul 3, 2024

2024-07-03 10:55:39 UTC Pre-commit check for 96bdac2 has started.
2024-07-03 10:58:57 UTC Build linux-x86_64-release-asan is running...
🔴 2024-07-03 11:21:45 UTC Build failed. see the build logs.
🔴 2024-07-03 11:21:46 UTC Tests run skipped.

Comment on lines 381 to 382
char buf[MaxNumberBytes];
result += SerializeNumber(payload.size(), buf);
Copy link
Contributor

Choose a reason for hiding this comment

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

We can compute count of needed bytes faster:
protocolbuffers/protobuf@7315f6d

Copy link
Contributor

Choose a reason for hiding this comment

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

#if (defined(__x86__) || defined(__x86_64__) || defined(_M_IX86) || \
     defined(_M_X64)) &&                                            \
    !(defined(__LZCNT__) || defined(__AVX2__))
// X86 CPUs lacking the lzcnt instruction are faster with the bsr-based
// implementation. MSVC does not define __LZCNT__, the nearest option that
// it interprets as lzcnt availability is __AVX2__.
#define EVENT_PB_PREFER_BSR 1
#else
#define EVENT_PB_PREFER_BSR 0
#endif

template<typename T>
inline size_t VarintSize(T value) {
#if EVENT_PB_PREFER_BSR
  // Explicit OR 0x1 to avoid calling std::countl_zero(0), which
  // requires a branch to check for on platforms without a clz instruction.
  uint32_t log2value = (std::numeric_limits<T>::digits - 1) -
                       std::countl_zero(value | 0x1);
  return static_cast<size_t>((log2value * 9 + (64 + 9)) / 64);
#else
  uint32_t clz = std::countl_zero(value);
  return static_cast<size_t>(
      ((std::numeric_limits<T>::digits * 9 + 64) - (clz * 9)) / 64);
#endif
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you for advice, but it looks it is bette to rewrite the whole file using google::protobuf::io::CodedInputStream without manual serialization/deserialization.

But I don't have the strength for it.

@github-actions
Copy link

github-actions bot commented Jul 3, 2024

2024-07-03 13:38:39 UTC Pre-commit check for 7f831fc has started.
2024-07-03 13:42:09 UTC Build linux-x86_64-release-clang14 is running...
🔴 2024-07-03 13:53:41 UTC Build failed. see the build logs.

@github-actions
Copy link

github-actions bot commented Jul 3, 2024

2024-07-03 13:56:03 UTC Pre-commit check for 7f831fc has started.
2024-07-03 13:59:34 UTC Build linux-x86_64-release-asan is running...
🔴 2024-07-03 14:13:28 UTC Build failed. see the build logs.
🔴 2024-07-03 14:13:29 UTC Tests run skipped.

@github-actions
Copy link

github-actions bot commented Jul 3, 2024

2024-07-03 14:06:32 UTC Pre-commit check for 7f831fc has started.
2024-07-03 14:10:06 UTC Build linux-x86_64-relwithdebinfo is running...
🔴 2024-07-03 14:23:44 UTC Build failed. see the build logs.
🔴 2024-07-03 14:23:46 UTC Tests run skipped.

@github-actions
Copy link

github-actions bot commented Jul 3, 2024

2024-07-03 15:11:15 UTC Pre-commit check for 31ca5fd has started.
2024-07-03 15:14:50 UTC Build linux-x86_64-release-asan is running...
🟢 2024-07-03 16:09:56 UTC Build successful.
2024-07-03 16:10:08 UTC Tests are running...
🔴 2024-07-03 19:38:36 UTC Some tests failed, follow the links below.

Test history | Test log

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
13784 12689 0 481 360 254

🟢 2024-07-03 19:39:43 UTC ydbd size 5.3 GiB changed* by -98.7 MiB, which is <= 0 Bytes vs main: OK

ydbd size dash main: bd87563 merge: 31ca5fd diff diff %
ydbd size 5 814 908 560 Bytes 5 711 457 808 Bytes -98.7 MiB -1.779%
ydbd stripped size 1 247 931 016 Bytes 1 235 242 600 Bytes -12.1 MiB -1.017%

*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation

@github-actions
Copy link

github-actions bot commented Jul 3, 2024

2024-07-03 16:12:24 UTC Pre-commit check for 31ca5fd has started.
2024-07-03 16:15:57 UTC Build linux-x86_64-release-clang14 is running...
🟢 2024-07-03 17:01:42 UTC Build successful.

@github-actions
Copy link

github-actions bot commented Jul 3, 2024

2024-07-03 16:19:13 UTC Pre-commit check for 31ca5fd has started.
2024-07-03 16:22:29 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-07-03 17:24:25 UTC Build successful.
2024-07-03 17:24:39 UTC Tests are running...
🔴 2024-07-03 20:45:23 UTC Some tests failed, follow the links below.

Test history | Test log

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
74812 60455 0 863 13380 114

🟢 2024-07-03 20:46:57 UTC ydbd size 8.2 GiB changed* by -146.0 MiB, which is <= 0 Bytes vs main: OK

ydbd size dash main: bd87563 merge: 31ca5fd diff diff %
ydbd size 8 926 807 928 Bytes 8 773 677 208 Bytes -146.0 MiB -1.715%
ydbd stripped size 486 814 568 Bytes 482 657 192 Bytes -4.0 MiB -0.854%

*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation

@github-actions
Copy link

github-actions bot commented Jul 8, 2024

2024-07-08 18:15:28 UTC Pre-commit check for 1f53979 has started.
2024-07-08 18:18:49 UTC Build linux-x86_64-release-clang14 is running...
🟢 2024-07-08 19:03:34 UTC Build successful.

@github-actions
Copy link

github-actions bot commented Jul 8, 2024

2024-07-08 18:17:20 UTC Pre-commit check for 1f53979 has started.
2024-07-08 18:20:41 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-07-08 19:25:22 UTC Build successful.
2024-07-08 19:25:41 UTC Tests are running...
🔴 2024-07-08 21:27:47 UTC Some tests failed, follow the links below.

Test history | Test log

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
74919 61464 0 3 13429 23

🟢 2024-07-08 21:28:24 UTC ydbd size 8.1 GiB changed* by -1.1 MiB, which is <= 0 Bytes vs main: OK

ydbd size dash main: 40256f3 merge: 1f53979 diff diff %
ydbd size 8 709 642 400 Bytes 8 708 516 648 Bytes -1.1 MiB -0.013%
ydbd stripped size 475 602 808 Bytes 475 586 808 Bytes -15.6 KiB -0.003%

*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation

@github-actions
Copy link

github-actions bot commented Jul 8, 2024

2024-07-08 18:45:03 UTC Pre-commit check for 1f53979 has started.
2024-07-08 18:48:37 UTC Build linux-x86_64-release-asan is running...
🟢 2024-07-08 19:41:10 UTC Build successful.
2024-07-08 19:41:24 UTC Tests are running...
🔴 2024-07-08 21:45:47 UTC Some tests failed, follow the links below.

Test history | Test log

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
13794 13173 0 102 343 176

🟢 2024-07-08 21:46:32 UTC ydbd size 5.2 GiB changed* by -560.5 KiB, which is <= 0 Bytes vs main: OK

ydbd size dash main: 40256f3 merge: 1f53979 diff diff %
ydbd size 5 606 258 672 Bytes 5 605 684 704 Bytes -560.5 KiB -0.010%
ydbd stripped size 1 206 562 200 Bytes 1 206 525 080 Bytes -36.2 KiB -0.003%

*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation

@maximyurchuk maximyurchuk marked this pull request as ready for review July 9, 2024 06:03
@maximyurchuk maximyurchuk merged commit 5b12fa6 into ydb-platform:main Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants