[improvement](memory) disable page cache and chunk allocator, optimize memory allocate size #13285

yiguolei · 2022-10-11T08:07:28Z

Proposed changes

disable page cache by default
disable chunk allocator by default
not use chunk allocator for vectorized allocator by default
add a new config memory_linear_growth_threshold = 128Mb, not allocate memory by RoundUpToPowerOf2 if the allocated size is larger than this threshold. This config is added to MemPool, ChunkAllocator, PodArray, Arena.

Problem summary

Describe your changes.

Checklist(Required)

Does it affect the original behavior:
- Yes
- No
- I don't know
Has unit tests been added:
- Yes
- No
- No Need
Has document been added or modified:
- Yes
- No
- No Need
Does it need to update dependencies:
- Yes
- No
Are there any changes that cannot be rolled back:
- Yes (If Yes, please explain WHY)
- No

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...

xinyiZzz · 2022-10-11T14:55:47Z

Gives the performance impact of disable page cache and disable chunk allocator, and the benefits of doing so, such as increasing OOM risk because they have no gc.

xinyiZzz · 2022-10-11T15:19:36Z

If want to discard the doris application layer cache. what is the replacement method?
such as, using optimized code + general memory allocator + system page cache

There are more application layer caches in doris, such as segment cache.

I think a self-managed application layer cache is necessary.
I think ultimately, an efficient and unified self-controllable cache is needed, rather than relying entirely on the system.

xinyiZzz · 2022-10-11T15:36:32Z

Maybe we can discuss the idea of subsequent cache optimization~, I mentioned a proposal a long time ago
#9580

yiguolei · 2022-10-12T07:09:46Z

Gives the performance impact of disable page cache and disable chunk allocator, and the benefits of doing so, such as increasing OOM risk because they have no gc.

Currently, we use a hard limit for page cache(20%) or chunk allocator(10%), if the application need memory, there is no gc mechanism to collect memory from page cache or chunk allocator.
page cache is very useful in some cases such as POC test, run benchmarks, some reporting secenarios. But in some cases like adhoc query, etl query, it is not very useful. And we have disable page cache in 1.1 and many company have disable page cache online such as xiaomi.
chunk allocator impacts about 10% performance in clickbench test.

Doris's memory usage is not very stable, I want to disable them first, and try to fix other memory problems and then open them again. This may need about 2 months and there are many releases during this stage, so that I disable them like we have done in branch 1.1-lts.

xinyiZzz · 2022-10-13T03:04:42Z

Gives the performance impact of disable page cache and disable chunk allocator, and the benefits of doing so, such as increasing OOM risk because they have no gc.

Currently, we use a hard limit for page cache(20%) or chunk allocator(10%), if the application need memory, there is no gc mechanism to collect memory from page cache or chunk allocator.

page cache is very useful in some cases such as POC test, run benchmarks, some reporting secenarios. But in some cases like adhoc query, etl query, it is not very useful. And we have disable page cache in 1.1 and many company have disable page cache online such as xiaomi.

chunk allocator impacts about 10% performance in clickbench test.

Doris's memory usage is not very stable, I want to disable them first, and try to fix other memory problems and then open them again. This may need about 2 months and there are many releases during this stage, so that I disable them like we have done in branch 1.1-lts.

I agree, page cache and chunk allocator may hide memory inelegant use on the code.

Later we can find a time to talk about the use of the cache

be/src/runtime/exec_env_init.cpp

xinyiZzz · 2022-10-13T10:34:10Z

be/src/runtime/exec_env_init.cpp

        return Status::InternalError(ss.str());
    }
-    chunk_reserved_bytes_limit =
-            BitUtil::RoundDown(chunk_reserved_bytes_limit, config::min_chunk_reserved_bytes);


The BitUtil::RoundDown(chunk_reserved_bytes_limit, 4096) here ensures that chunk_reserved_bytes_limit is a multiple of 4096

4096 is the minimum chunk size currently allocated by the chunk allocator

A separate conf min_chunk_reserved_bytes is not necessary, but RoundDown is meaningful

yes ... I will move back min_chunk_reserved_bytes

can remove min_chunk_reserved_bytes, const 4096 is fine, the user will not modify it
This knowledge suggests~

OK， I remove it.

be/src/runtime/mem_pool.cpp

be/src/runtime/mem_pool.h

be/src/vec/common/arena.h

xinyiZzz · 2022-10-13T11:06:09Z

be/src/vec/common/pod_array.h

+    /// Not round up, keep the size just as the application pass in like std::vector
    void alloc_for_num_elements(size_t num_elements) {
-        alloc(round_up_to_power_of_two_or_zero(minimum_memory_for_elements(num_elements)));
+        alloc(minimum_memory_for_elements(num_elements));


Allocating in powers of 2 has a positive impact on performance, if you wish to reduce memory usage,

join #ifndef STRICT_MEMORY_USE, similar to hash_table.h expansion

I do not think this should be a config, because if it is a config, we do not know when to open the config since it is a macro. Actually, there are two types of memory allocation in PODArray:

reserve, sometimes the developer know the expected size of the array, then he should call reserve method to allocate the EXPECTED memory.

push_back, the developer does not know the expected size of the array, then he just call push back to allocate memory. In this scenario, we should allocate memory using power of 2.

For most cases, we should reserve or resize memory size before push back, then we could reduce memory reallocation or memory copy.
This PR try to fix some problems. #13088.

be/src/vec/common/pod_array.h

xinyiZzz · 2022-10-13T11:11:46Z

be/src/common/config.h

-CONF_Int32(min_chunk_reserved_bytes, "1024");
+
+// Whether using chunk allocator to cache memory chunk
+CONF_Bool(disable_chunk_allocator, "true");


disable_chunk_allocator_in_mem_pool

No, I will remove mempool after we removed non-vectorized engine. MemPool is used as MemPool.cpp, it is like a arena. The config disable_mem_pools is also very confused. I will remove them.

xinyiZzz · 2022-10-13T11:13:00Z

be/src/runtime/memory/chunk_allocator.cpp

    DCHECK(chunk.core_id != -1);
    CHECK((chunk.size & (chunk.size - 1)) == 0);
-    if (config::disable_mem_pools) {
+    if (config::disable_chunk_allocator) {


Is it better to move the condition into MemPool and rename disable_chunk_allocator_in_mem_pool, Similar to disable_chunk_allocator_in_vec

Not use this. I will remove disable_mem_pools in the future after non-vectorized engine is removed. It is very confused with MemPool.

Try removing disable_chunk_allocator_in_vec and replace with disable_chunk_allocator. (more detailed config comments)

…e memory allocate size

xinyiZzz

LGTM

github-actions · 2022-10-15T09:25:23Z

PR approved by at least one committer and no changes requested.

github-actions · 2022-10-15T09:25:26Z

PR approved by anyone and no changes requested.

… optimize memory allocate size (apache#13285)" This reverts commit a5f3880.

) * Revert "[fix](mem) failure of allocating memory (#13414)" This reverts commit 971eb91. * Revert "[improvement](memory) disable page cache and chunk allocator, optimize memory allocate size (#13285)" This reverts commit a5f3880.

) * Revert "[fix](mem) failure of allocating memory (#13414)" This reverts commit 971eb91. * Revert "[improvement](memory) disable page cache and chunk allocator, optimize memory allocate size (#13285)" This reverts commit a5f3880. Conflicts: be/src/common/config.h

github-actions bot added the area/vectorization label Oct 11, 2022

yiguolei force-pushed the forbidden_pagecache_default branch from 8b7d928 to 95f4830 Compare October 13, 2022 03:34

yiguolei added the dev/1.1.4-deprecated label Oct 13, 2022

xinyiZzz reviewed Oct 13, 2022

View reviewed changes

yiguolei force-pushed the forbidden_pagecache_default branch from e906afb to eb659d1 Compare October 14, 2022 06:11

Doris-Extras added 6 commits October 15, 2022 10:22

[improvement](memory) disable page cache and chunk allocator, optimiz…

c6d083a

…e memory allocate size

fix bugs

2ced248

fix bugs

5e1cc77

fix bugs

afe8c8d

fix bugs

3051496

fix bugs

f8ad6c0

yiguolei force-pushed the forbidden_pagecache_default branch from eb659d1 to f8ad6c0 Compare October 15, 2022 02:30

xinyiZzz approved these changes Oct 15, 2022

View reviewed changes

github-actions bot added the approved Indicates a PR has been approved by one committer. label Oct 15, 2022

github-actions bot added the reviewed label Oct 15, 2022

xinyiZzz merged commit a5f3880 into apache:master Oct 15, 2022

Gabriel39 added a commit to Gabriel39/incubator-doris that referenced this pull request Oct 18, 2022

Revert "[improvement](memory) disable page cache and chunk allocator,…

18f4f61

… optimize memory allocate size (apache#13285)" This reverts commit a5f3880.

yiguolei removed the dev/1.1.4-deprecated label Oct 19, 2022

yiguolei mentioned this pull request Oct 19, 2022

[memory](podarray) revert not allocate too much memory in podarray change #13457

Merged

13 tasks

HappenLee added a commit to HappenLee/incubator-doris that referenced this pull request Oct 19, 2022

Revert "[improvement](memory) disable page cache and chunk allocator,…

c7f2610

… optimize memory allocate size (apache#13285)" This reverts commit a5f3880.

HappenLee added a commit to HappenLee/incubator-doris that referenced this pull request Oct 20, 2022

Revert "[improvement](memory) disable page cache and chunk allocator,…

515c6c1

… optimize memory allocate size (apache#13285)" This reverts commit a5f3880.

xinyiZzz mentioned this pull request Oct 20, 2022

[Revert](mem) revert the mem config cause perfermace degradation #13526

Merged

13 tasks

morningman mentioned this pull request Nov 21, 2022

Release Note 1.2.0 #14461

Closed

yiguolei deleted the forbidden_pagecache_default branch March 30, 2023 10:19

[improvement](memory) disable page cache and chunk allocator, optimize memory allocate size #13285

[improvement](memory) disable page cache and chunk allocator, optimize memory allocate size #13285

Uh oh!

Conversation

yiguolei commented Oct 11, 2022

Proposed changes

Problem summary

Checklist(Required)

Further comments

Uh oh!

xinyiZzz commented Oct 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xinyiZzz commented Oct 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xinyiZzz commented Oct 11, 2022

Uh oh!

yiguolei commented Oct 12, 2022

Uh oh!

xinyiZzz commented Oct 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xinyiZzz Oct 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xinyiZzz left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Oct 15, 2022

Uh oh!

github-actions bot commented Oct 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

xinyiZzz commented Oct 11, 2022 •

edited

Loading

xinyiZzz commented Oct 11, 2022 •

edited

Loading

xinyiZzz commented Oct 13, 2022 •

edited

Loading

xinyiZzz Oct 13, 2022 •

edited

Loading