Skip to content

Conversation

@adonis0147
Copy link
Contributor

Proposed changes

Issue Number: close #33570

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@doris-robot
Copy link

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

@adonis0147 adonis0147 marked this pull request as draft April 19, 2024 09:37
}
namespace doris {

int posix_memalign(void** r, size_t a, size_t s) {
Copy link
Contributor

Choose a reason for hiding this comment

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

hook is useless now

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

#include <features.h>
#include <stdint.h>
#include <stdlib.h>
#include <jemalloc/jemalloc.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: 'jemalloc/jemalloc.h' file not found [clang-diagnostic-error]

#include <jemalloc/jemalloc.h>
         ^

Comment on lines +26 to +27
#define HOOK_MAX 4

Copy link
Contributor

Choose a reason for hiding this comment

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

warning: replace macro with enum [modernize-macro-to-enum]

Suggested change
#define HOOK_MAX 4
enum {
HOOK_MAX = 4};

void* doris_realloc(void* p, size_t size) __THROW {
if (UNLIKELY(size == 0)) {
return nullptr;
#define HOOK_MAX 4
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: macro 'HOOK_MAX' defines an integral constant; prefer an enum instead [modernize-macro-to-enum]

#define HOOK_MAX 4
        ^

void* extra;
};

void doris_hook_alloc(void* extra, hook_alloc_t type, void* result, uintptr_t result_raw,
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: function 'doris_hook_alloc' has cognitive complexity of 76 (threshold 50) [readability-function-cognitive-complexity]

void doris_hook_alloc(void* extra, hook_alloc_t type, void* result, uintptr_t result_raw,
     ^
Additional context

be/src/runtime/memory/jemalloc_hook.cpp:86: +1, including nesting penalty of 0, nesting level increased to 1

    if (result == nullptr) {
    ^

be/src/runtime/memory/jemalloc_hook.cpp:89: +1, including nesting penalty of 0, nesting level increased to 1

    switch (type) {
    ^

be/src/runtime/memory/jemalloc_hook.cpp:92: +2, including nesting penalty of 1, nesting level increased to 2

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN([](size_t size) { return nallocx(size, 0); },
        ^

be/src/runtime/thread_context.h:433: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN'

    do {                                                                   \
    ^

be/src/runtime/memory/jemalloc_hook.cpp:92: +3, including nesting penalty of 2, nesting level increased to 3

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN([](size_t size) { return nallocx(size, 0); },
        ^

be/src/runtime/thread_context.h:434: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN'

        if (doris::use_mem_hook) {                                         \
        ^

be/src/runtime/memory/jemalloc_hook.cpp:92: nesting level increased to 4

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN([](size_t size) { return nallocx(size, 0); },
                                                   ^

be/src/runtime/memory/jemalloc_hook.cpp:98: +2, including nesting penalty of 1, nesting level increased to 2

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK(size);
        ^

be/src/runtime/thread_context.h:426: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK'

    do {                                                   \
    ^

be/src/runtime/memory/jemalloc_hook.cpp:98: +3, including nesting penalty of 2, nesting level increased to 3

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK(size);
        ^

be/src/runtime/thread_context.h:427: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK'

        if (doris::use_mem_hook) {                         \
        ^

be/src/runtime/memory/jemalloc_hook.cpp:99: +2, including nesting penalty of 1, nesting level increased to 2

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN(
        ^

be/src/runtime/thread_context.h:433: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN'

    do {                                                                   \
    ^

be/src/runtime/memory/jemalloc_hook.cpp:99: +3, including nesting penalty of 2, nesting level increased to 3

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN(
        ^

be/src/runtime/thread_context.h:434: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN'

        if (doris::use_mem_hook) {                                         \
        ^

be/src/runtime/memory/jemalloc_hook.cpp:100: nesting level increased to 4

                [](void* ptr, size_t size) { return malloc_usable_size(ptr) - size; }, result,
                ^

be/src/runtime/memory/jemalloc_hook.cpp:106: +2, including nesting penalty of 1, nesting level increased to 2

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK(size);
        ^

be/src/runtime/thread_context.h:426: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK'

    do {                                                   \
    ^

be/src/runtime/memory/jemalloc_hook.cpp:106: +3, including nesting penalty of 2, nesting level increased to 3

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK(size);
        ^

be/src/runtime/thread_context.h:427: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK'

        if (doris::use_mem_hook) {                         \
        ^

be/src/runtime/memory/jemalloc_hook.cpp:107: +2, including nesting penalty of 1, nesting level increased to 2

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN(
        ^

be/src/runtime/thread_context.h:433: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN'

    do {                                                                   \
    ^

be/src/runtime/memory/jemalloc_hook.cpp:107: +3, including nesting penalty of 2, nesting level increased to 3

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN(
        ^

be/src/runtime/thread_context.h:434: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN'

        if (doris::use_mem_hook) {                                         \
        ^

be/src/runtime/memory/jemalloc_hook.cpp:108: nesting level increased to 4

                [](void* ptr, size_t size) { return malloc_usable_size(ptr) - size; }, result,
                ^

be/src/runtime/memory/jemalloc_hook.cpp:115: +2, including nesting penalty of 1, nesting level increased to 2

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK(num * size);
        ^

be/src/runtime/thread_context.h:426: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK'

    do {                                                   \
    ^

be/src/runtime/memory/jemalloc_hook.cpp:115: +3, including nesting penalty of 2, nesting level increased to 3

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK(num * size);
        ^

be/src/runtime/thread_context.h:427: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK'

        if (doris::use_mem_hook) {                         \
        ^

be/src/runtime/memory/jemalloc_hook.cpp:116: +2, including nesting penalty of 1, nesting level increased to 2

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN(
        ^

be/src/runtime/thread_context.h:433: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN'

    do {                                                                   \
    ^

be/src/runtime/memory/jemalloc_hook.cpp:116: +3, including nesting penalty of 2, nesting level increased to 3

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN(
        ^

be/src/runtime/thread_context.h:434: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN'

        if (doris::use_mem_hook) {                                         \
        ^

be/src/runtime/memory/jemalloc_hook.cpp:117: nesting level increased to 4

                [](void* ptr, size_t size) { return malloc_usable_size(ptr) - size; }, result,
                ^

be/src/runtime/memory/jemalloc_hook.cpp:123: +2, including nesting penalty of 1, nesting level increased to 2

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK(size);
        ^

be/src/runtime/thread_context.h:426: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK'

    do {                                                   \
    ^

be/src/runtime/memory/jemalloc_hook.cpp:123: +3, including nesting penalty of 2, nesting level increased to 3

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK(size);
        ^

be/src/runtime/thread_context.h:427: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK'

        if (doris::use_mem_hook) {                         \
        ^

be/src/runtime/memory/jemalloc_hook.cpp:124: +2, including nesting penalty of 1, nesting level increased to 2

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN(
        ^

be/src/runtime/thread_context.h:433: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN'

    do {                                                                   \
    ^

be/src/runtime/memory/jemalloc_hook.cpp:124: +3, including nesting penalty of 2, nesting level increased to 3

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN(
        ^

be/src/runtime/thread_context.h:434: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN'

        if (doris::use_mem_hook) {                                         \
        ^

be/src/runtime/memory/jemalloc_hook.cpp:125: nesting level increased to 4

                [](void* ptr, size_t size) { return malloc_usable_size(ptr) - size; }, result,
                ^

be/src/runtime/memory/jemalloc_hook.cpp:131: +2, including nesting penalty of 1, nesting level increased to 2

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK(size);
        ^

be/src/runtime/thread_context.h:426: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK'

    do {                                                   \
    ^

be/src/runtime/memory/jemalloc_hook.cpp:131: +3, including nesting penalty of 2, nesting level increased to 3

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK(size);
        ^

be/src/runtime/thread_context.h:427: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK'

        if (doris::use_mem_hook) {                         \
        ^

be/src/runtime/memory/jemalloc_hook.cpp:132: +2, including nesting penalty of 1, nesting level increased to 2

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN(
        ^

be/src/runtime/thread_context.h:433: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN'

    do {                                                                   \
    ^

be/src/runtime/memory/jemalloc_hook.cpp:132: +3, including nesting penalty of 2, nesting level increased to 3

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN(
        ^

be/src/runtime/thread_context.h:434: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN'

        if (doris::use_mem_hook) {                                         \
        ^

be/src/runtime/memory/jemalloc_hook.cpp:133: nesting level increased to 4

                [](void* ptr, size_t size) { return malloc_usable_size(ptr) - size; }, result,
                ^

be/src/runtime/memory/jemalloc_hook.cpp:140: +2, including nesting penalty of 1, nesting level increased to 2

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN([](size_t size) { return nallocx(size, 0); },
        ^

be/src/runtime/thread_context.h:433: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN'

    do {                                                                   \
    ^

be/src/runtime/memory/jemalloc_hook.cpp:140: +3, including nesting penalty of 2, nesting level increased to 3

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN([](size_t size) { return nallocx(size, 0); },
        ^

be/src/runtime/thread_context.h:434: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN'

        if (doris::use_mem_hook) {                                         \
        ^

be/src/runtime/memory/jemalloc_hook.cpp:140: nesting level increased to 4

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN([](size_t size) { return nallocx(size, 0); },
                                                   ^

be/src/runtime/memory/jemalloc_hook.cpp:142: +2, including nesting penalty of 1, nesting level increased to 2

        if (flag) {
        ^

be/src/runtime/memory/jemalloc_hook.cpp:143: +3, including nesting penalty of 2, nesting level increased to 3

            CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN(
            ^

be/src/runtime/thread_context.h:433: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN'

    do {                                                                   \
    ^

be/src/runtime/memory/jemalloc_hook.cpp:143: +4, including nesting penalty of 3, nesting level increased to 4

            CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN(
            ^

be/src/runtime/thread_context.h:434: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN'

        if (doris::use_mem_hook) {                                         \
        ^

be/src/runtime/memory/jemalloc_hook.cpp:144: nesting level increased to 5

                    [](void* ptr, size_t size) { return malloc_usable_size(ptr) - size; }, result,
                    ^

be/src/runtime/memory/jemalloc_hook.cpp:151: +2, including nesting penalty of 1, nesting level increased to 2

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN([](size_t size) { return nallocx(size, 0); },
        ^

be/src/runtime/thread_context.h:433: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN'

    do {                                                                   \
    ^

be/src/runtime/memory/jemalloc_hook.cpp:151: +3, including nesting penalty of 2, nesting level increased to 3

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN([](size_t size) { return nallocx(size, 0); },
        ^

be/src/runtime/thread_context.h:434: expanded from macro 'CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN'

        if (doris::use_mem_hook) {                                         \
        ^

be/src/runtime/memory/jemalloc_hook.cpp:151: nesting level increased to 4

        CONSUME_THREAD_MEM_TRACKER_BY_HOOK_WITH_FN([](size_t size) { return nallocx(size, 0); },
                                                   ^

};

void doris_hook_alloc(void* extra, hook_alloc_t type, void* result, uintptr_t result_raw,
uintptr_t args_raw[3]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: pointer parameter 'args_raw' can be pointer to const [readability-non-const-parameter]

Suggested change
uintptr_t args_raw[3]) {
const uintptr_t args_raw[3]) {

xinyiZzz added a commit that referenced this pull request May 10, 2024
…#34578)

Now in order to support malloc hook, compiling jemalloc will add tow compilation options --with-jemalloc-prefix=je and --disable-cxx, then overwrite malloc/free as follows to allow BE to use jemalloc:

#define ALIAS(doris_fn) __attribute__((alias(#doris_fn), used))
void* malloc(size_t size) __THROW ALIAS(doris_malloc);
void free(void* p) __THROW ALIAS(doris_free);
void* realloc(void* p, size_t size) __THROW ALIAS(doris_realloc);
void* calloc(size_t n, size_t size) __THROW ALIAS(doris_calloc);
void cfree(void* ptr) __THROW ALIAS(doris_cfree);
but after such overwrite like this, Doris BE on jdk17 cannot be started in some environments.
there are three solutions:

Modify overwrite malloc/free, fixed in Use jemalloc's experimental features to hook jemalloc #33897
Modify BE dynamically loads JVM, fixed in [Fix](jdk) Fix jdk17 crash on some envs. #32865
Commit of this PR, modify jemalloc compilation options, after remove --with-jemalloc-prefix=je, there is not need to overwrite malloc/free, BE will use Jemalloc by default.
So, if need to use memory hook, this PR still cannot solve this problem.
Looking forward to modify BE dynamically loads JVM, which will not only solve this problem, also fix other crashes caused by BE use JVM.
xinyiZzz added a commit to xinyiZzz/incubator-doris that referenced this pull request May 11, 2024
…apache#34578)

Now in order to support malloc hook, compiling jemalloc will add tow compilation options --with-jemalloc-prefix=je and --disable-cxx, then overwrite malloc/free as follows to allow BE to use jemalloc:

void* malloc(size_t size) __THROW ALIAS(doris_malloc);
void free(void* p) __THROW ALIAS(doris_free);
void* realloc(void* p, size_t size) __THROW ALIAS(doris_realloc);
void* calloc(size_t n, size_t size) __THROW ALIAS(doris_calloc);
void cfree(void* ptr) __THROW ALIAS(doris_cfree);
but after such overwrite like this, Doris BE on jdk17 cannot be started in some environments.
there are three solutions:

Modify overwrite malloc/free, fixed in Use jemalloc's experimental features to hook jemalloc apache#33897
Modify BE dynamically loads JVM, fixed in [Fix](jdk) Fix jdk17 crash on some envs. apache#32865
Commit of this PR, modify jemalloc compilation options, after remove --with-jemalloc-prefix=je, there is not need to overwrite malloc/free, BE will use Jemalloc by default.
So, if need to use memory hook, this PR still cannot solve this problem.
Looking forward to modify BE dynamically loads JVM, which will not only solve this problem, also fix other crashes caused by BE use JVM.
ByteYue pushed a commit to ByteYue/doris that referenced this pull request May 15, 2024
…apache#34578)

Now in order to support malloc hook, compiling jemalloc will add tow compilation options --with-jemalloc-prefix=je and --disable-cxx, then overwrite malloc/free as follows to allow BE to use jemalloc:

#define ALIAS(doris_fn) __attribute__((alias(#doris_fn), used))
void* malloc(size_t size) __THROW ALIAS(doris_malloc);
void free(void* p) __THROW ALIAS(doris_free);
void* realloc(void* p, size_t size) __THROW ALIAS(doris_realloc);
void* calloc(size_t n, size_t size) __THROW ALIAS(doris_calloc);
void cfree(void* ptr) __THROW ALIAS(doris_cfree);
but after such overwrite like this, Doris BE on jdk17 cannot be started in some environments.
there are three solutions:

Modify overwrite malloc/free, fixed in Use jemalloc's experimental features to hook jemalloc apache#33897
Modify BE dynamically loads JVM, fixed in [Fix](jdk) Fix jdk17 crash on some envs. apache#32865
Commit of this PR, modify jemalloc compilation options, after remove --with-jemalloc-prefix=je, there is not need to overwrite malloc/free, BE will use Jemalloc by default.
So, if need to use memory hook, this PR still cannot solve this problem.
Looking forward to modify BE dynamically loads JVM, which will not only solve this problem, also fix other crashes caused by BE use JVM.
@github-actions
Copy link
Contributor

We're closing this PR because it hasn't been updated in a while.
This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and feel free a maintainer to remove the Stale tag!

@github-actions github-actions bot added the Stale label Oct 17, 2024
@github-actions github-actions bot closed this Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Failed to start BE with JDK 17.

3 participants