Skip to content

Conversation

@kruall
Copy link
Collaborator

@kruall kruall commented May 15, 2024

Changelog entry

...

Changelog category

  • Bugfix

Additional information

...

@kruall kruall added the area/actorsystem Actor System related issues label May 15, 2024
@kruall kruall requested review from dcherednik and snaury and removed request for dcherednik May 15, 2024 13:08
@github-actions
Copy link

github-actions bot commented May 15, 2024

2024-05-15 13:12:06 UTC Pre-commit check for 702be4c has started.
2024-05-15 13:12:09 UTC Build linux-x86_64-release-asan is running...
🟢 2024-05-15 13:52:08 UTC Build successful.
2024-05-15 13:54:08 UTC Tests are running...
🔴 2024-05-15 14:27:58 UTC Test run completed, no test results found for commit 407562e. Please check build logs.
2024-05-15 14:28:01 UTC Check cancelled

@github-actions
Copy link

github-actions bot commented May 15, 2024

2024-05-15 13:12:11 UTC Pre-commit check for 702be4c has started.
2024-05-15 13:12:14 UTC Build linux-x86_64-release-clang14 is running...
🟢 2024-05-15 13:51:51 UTC Build successful.

@github-actions
Copy link

github-actions bot commented May 15, 2024

2024-05-15 13:12:11 UTC Pre-commit check for 702be4c has started.
2024-05-15 13:12:14 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-05-15 13:53:30 UTC Build successful.
2024-05-15 13:55:25 UTC Tests are running...
🔴 2024-05-15 14:28:06 UTC Test run completed, no test results found for commit 407562e. Please check build logs.
2024-05-15 14:28:10 UTC Check cancelled

reclaimAsFree = true;

NHPTimer::STime elapsed = Ctx.AddEventProcessingStats(hpprev, hpnow, activityType, CurrentActorScheduledEventsCounter);
if (Y_LIKELY(hpprev < hpnow)) {
Copy link
Member

Choose a reason for hiding this comment

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

Я думаю эта проверка должны быть внутри AddEventProcessingStats, чтобы мы например учитывать затраченное время как 0, но при этом увеличивать ReceivedEvents и т.д.

hpprev = TlsThreadContext->StartOfElapsingTime.exchange(hpnow, std::memory_order_acq_rel);
Ctx.AddElapsedCycles(ActorSystemIndex, hpnow - hpprev);
hpprev = TlsThreadContext->UpdateStartOfElapsingTime(hpnow);
if (Y_LIKELY(hpprev < hpnow)) {
Copy link
Member

Choose a reason for hiding this comment

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

Я бы предложил проверки на > 0 унести внутрь AddParkedCycles/AddElapsedCycles, чтобы не дублировать их по много раз.

ui64 UpdateStartOfElapsingTime(ui64 newValue) {
ui64 oldValue = StartOfElapsingTime.load(std::memory_order_acquire);
for (;;) {
if (newValue <= oldValue) {
Copy link
Member

Choose a reason for hiding this comment

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

С одной стороны tsc счётчика должно хватать на 190 лет (на частоте 3GHz), с другой стороны я не уверен начинается ли он всегда с нуля. Может быть вычислять newValue - oldValue в виде i64 и проверять что оно больше нуля? В генерируемом коде при этом разницы почти не будет, зато на случай overflow будет соломка.


NHPTimer::STime elapsed = Ctx.AddEventProcessingStats(hpprev, hpnow, activityType, CurrentActorScheduledEventsCounter);
if (Y_LIKELY(hpprev < hpnow)) {
Ctx.AddEventProcessingStats(hpprev, hpnow, activityType, CurrentActorScheduledEventsCounter);
Copy link
Member

Choose a reason for hiding this comment

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

И кстати в EventProcessingTime (гистограмму) мы должны добавлять hpnow - eventStart. Если другой поток при сборе статистики часть времени откусил, это не значит что обработка события вдруг стала быстрой.

@github-actions
Copy link

github-actions bot commented May 15, 2024

2024-05-15 14:29:28 UTC Pre-commit check for 66b5733 has started.
2024-05-15 14:29:29 UTC Build linux-x86_64-release-asan is running...
2024-05-15 14:32:30 UTC Check cancelled

@github-actions
Copy link

github-actions bot commented May 15, 2024

2024-05-15 14:29:36 UTC Pre-commit check for 66b5733 has started.
2024-05-15 14:29:38 UTC Build linux-x86_64-release-clang14 is running...
2024-05-15 14:32:30 UTC Check cancelled

@github-actions
Copy link

github-actions bot commented May 15, 2024

2024-05-15 14:30:50 UTC Pre-commit check for 66b5733 has started.
2024-05-15 14:30:52 UTC Build linux-x86_64-relwithdebinfo is running...
2024-05-15 14:32:29 UTC Check cancelled

@kruall kruall force-pushed the as/fix_walking_back_StartOfElapsingTime branch from d8776f2 to af87841 Compare May 15, 2024 14:32
@github-actions
Copy link

github-actions bot commented May 15, 2024

2024-05-15 14:33:38 UTC Pre-commit check for 8792ae7 has started.
2024-05-15 14:33:39 UTC Build linux-x86_64-relwithdebinfo is running...
2024-05-15 14:39:25 UTC Check cancelled

@github-actions
Copy link

github-actions bot commented May 15, 2024

2024-05-15 14:33:39 UTC Pre-commit check for 8792ae7 has started.
2024-05-15 14:33:40 UTC Build linux-x86_64-release-asan is running...
2024-05-15 14:39:24 UTC Check cancelled

@github-actions
Copy link

github-actions bot commented May 15, 2024

2024-05-15 14:35:44 UTC Pre-commit check for 8792ae7 has started.
2024-05-15 14:35:46 UTC Build linux-x86_64-release-clang14 is running...
2024-05-15 14:39:24 UTC Check cancelled

@github-actions
Copy link

github-actions bot commented May 15, 2024

2024-05-15 14:41:41 UTC Pre-commit check for 012a3a3 has started.
2024-05-15 14:41:43 UTC Build linux-x86_64-release-asan is running...
🟢 2024-05-15 15:22:10 UTC Build successful.
2024-05-15 15:23:52 UTC Tests are running...
🔴 2024-05-15 17:08:37 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
13518 13424 0 33 49 12

@github-actions
Copy link

github-actions bot commented May 15, 2024

2024-05-15 14:42:55 UTC Pre-commit check for 012a3a3 has started.
2024-05-15 14:42:57 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-05-15 15:22:26 UTC Build successful.
2024-05-15 15:24:24 UTC Tests are running...
🔴 2024-05-15 17:15:59 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
72419 59745 0 5 12657 12

@github-actions
Copy link

github-actions bot commented May 15, 2024

2024-05-15 14:44:26 UTC Pre-commit check for 012a3a3 has started.
2024-05-15 14:44:28 UTC Build linux-x86_64-release-clang14 is running...
🟢 2024-05-15 15:24:30 UTC Build successful.

@kruall kruall merged commit e884754 into ydb-platform:main May 16, 2024
@kruall kruall deleted the as/fix_walking_back_StartOfElapsingTime branch May 16, 2024 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/actorsystem Actor System related issues bugfix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants