Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: improve #2863

Draft
wants to merge 40 commits into
base: main
Choose a base branch
from
Draft

feat: improve #2863

wants to merge 40 commits into from

Conversation

beats-dh
Copy link
Collaborator

No description provided.

Copy link
Contributor

github-actions bot commented Aug 31, 2024

Qodana for C/C++

3092 new problems were found

Inspection name Severity Problems
convert-member-functions-to-static 🔶 Warning 813
misra-cpp2008-5-0-11 🔶 Warning 496
init-variables 🔶 Warning 375
easily-swappable-parameters 🔶 Warning 210
branch-clone 🔶 Warning 149
dcl58-cpp 🔶 Warning 129
misra-cpp2008-5-3-1 🔶 Warning 117
make-member-function-const 🔶 Warning 73
misra-cpp2008-5-0-12 🔶 Warning 68
function-cognitive-complexity 🔶 Warning 67
pro-type-reinterpret-cast 🔶 Warning 67
magic-numbers 🔶 Warning 66
pro-type-member-init 🔶 Warning 44
misra-cpp2008-0-1-7 🔶 Warning 42
inconsistent-declaration-parameter-name 🔶 Warning 35
implicit-bool-conversion 🔶 Warning 35
unused-parameters 🔶 Warning 35
pro-bounds-pointer-arithmetic 🔶 Warning 35
unroll-loops 🔶 Warning 32
misra-cpp2008-6-5-1 🔶 Warning 26
id-dependent-backward-branch 🔶 Warning 24
misra-cpp2008-6-2-1 🔶 Warning 19
simplify-boolean-expr 🔶 Warning 17
restrict-system-libc-headers 🔶 Warning 14
use-equals-default 🔶 Warning 10
named-parameter 🔶 Warning 9
trivially-destructible 🔶 Warning 7
qualified-auto 🔶 Warning 7
exception-baseclass 🔶 Warning 7
rvalue-reference-param-not-moved 🔶 Warning 6
no-recursion 🔶 Warning 6
misra-cpp2008-12-1-3 🔶 Warning 5
simd-intrinsics 🔶 Warning 5
misra-cpp2008-5-0-5 🔶 Warning 4
use-auto 🔶 Warning 4
misra-cpp2008-8-0-1 🔶 Warning 4
readability-casting 🔶 Warning 3
misra-cpp2008-2-13-3 🔶 Warning 3
prefer-member-initializer 🔶 Warning 3
misra-cpp2008-5-0-6 🔶 Warning 2
redundant-casting 🔶 Warning 2
misra-cpp2008-3-1-2 🔶 Warning 2
enum-size 🔶 Warning 1
redundant-member-init 🔶 Warning 1
misra-cpp2008-5-0-13 🔶 Warning 1
misra-cpp2008-5-14-1 🔶 Warning 1
misra-cpp2008-5-2-8 🔶 Warning 1
narrowing-conversions 🔶 Warning 1
empty-catch 🔶 Warning 1
chained-comparison 🔶 Warning 1
statically-constructed-objects 🔶 Warning 1
isolate-declaration 🔶 Warning 1
misra-cpp2008-6-3-1 🔶 Warning 1
misra-cpp2008-7-3-4 🔶 Warning 1
build-using-namespace 🔶 Warning 1
readability-todo 🔶 Warning 1
static-accessed-through-instance 🔶 Warning 1

💡 Qodana analysis was run in the pull request mode: only the changed files were checked

View the detailed Qodana report

To be able to view the detailed Qodana report, you can either:

To get *.log files or any other Qodana artifacts, run the action with upload-result option set to true,
so that the action will upload the files as the job artifacts:

      - name: 'Qodana Scan'
        uses: JetBrains/qodana-action@v2024.1.9
        with:
          upload-result: true
Contact Qodana team

Contact us at qodana-support@jetbrains.com

@beats-dh beats-dh force-pushed the beats-fixs branch 3 times, most recently from b954f4f to 4d0a313 Compare September 2, 2024 04:57
@@ -135,7 +135,7 @@ void Dispatcher::executeScheduledEvents() {

if (task->execute() && task->isCycle()) {
task->updateTime();
threadScheduledTasks.emplace_back(task);
threadScheduledTasks.emplace(task);
Copy link
Contributor

Choose a reason for hiding this comment

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

rollback

if (!thread->tasks[serial].empty()) {
m_tasks[serial].insert(m_tasks[serial].end(), make_move_iterator(thread->tasks[serial].begin()), make_move_iterator(thread->tasks[serial].end()));
thread->tasks[serial].clear();
{
Copy link
Contributor

Choose a reason for hiding this comment

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

rollback

@@ -207,12 +207,11 @@ class Dispatcher {
}

std::array<std::vector<Task>, static_cast<uint8_t>(TaskGroup::Last)> tasks;
std::vector<std::shared_ptr<Task>> scheduledTasks;
phmap::parallel_flat_hash_set_m<std::shared_ptr<Task>> scheduledTasks;
Copy link
Contributor

Choose a reason for hiding this comment

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

rollback

@beats-dh beats-dh force-pushed the beats-fixs branch 2 times, most recently from ef5cdf0 to d3718ac Compare September 12, 2024 00:00
src/utils/lockfree.hpp Outdated Show resolved Hide resolved
src/utils/lockfree.hpp Outdated Show resolved Hide resolved
src/utils/lockfree.hpp Outdated Show resolved Hide resolved
src/utils/lockfree.hpp Outdated Show resolved Hide resolved
@beats-dh
Copy link
Collaborator Author

beats-dh commented Sep 13, 2024

@jhogberg
I made a new implementation using the atomic_queue (https://github.com/max0x7ba/atomic_queue) lib, take a look!
https://github.com/opentibiabr/canary/blob/beats-fixs/src/utils/lockfree.hpp

}
return static_cast<T*>(p);
return static_cast<T*>(::operator new(n * sizeof(T), static_cast<std::align_val_t>(alignof(T))));

Choose a reason for hiding this comment

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

Why not array-new? new[n] T(…)

@@ -8,114 +8,62 @@
*/

#pragma once
#include <atomic_queue/atomic_queue.h>

Choose a reason for hiding this comment

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

Do we need this? It introduces another layer of complexity that I have a hard time seeing a need for, given how this is currently used.

The per-thread caches are a great idea on their own even without this layer. Is there a reason for the buffers to be recycled across threads?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants