You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/07/20 09:51:47 UTC

[GitHub] [doris] muyizi opened a new pull request, #11040: fix_arena_push_size

muyizi opened a new pull request, #11040:
URL: https://github.com/apache/doris/pull/11040

   Chunk* c1 = ChunkAllocator::allocate(1025);
   ChunkAllocator::free(c1);
   ChunkAllocator::allocate(2000); //会返回之前的chunk,这是不符合预期的
   
   修改后,如果申请1025的话,就是从free_list[11]中取,放回free_list[10]里面去;free_list[11]里面应该放size >= 2048 && size < 4096的chunk,以此保证申请到的size大于需要申请的size.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] muyizi closed pull request #11040: fix_arena_push_size

Posted by GitBox <gi...@apache.org>.
muyizi closed pull request #11040: fix_arena_push_size
URL: https://github.com/apache/doris/pull/11040


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] muyizi commented on pull request #11040: fix_arena_push_size

Posted by GitBox <gi...@apache.org>.
muyizi commented on PR #11040:
URL: https://github.com/apache/doris/pull/11040#issuecomment-1195266784

   > 如果 ChunkAllocator 中的 chunk 支持不定长内存的话,那预期存在内存浪费问题,
   > 
   > 按之前的逻辑,申请1624k会实际申请2048k,free时放回2048k的free list中,在之后满足1024k到2048k的内存申请
   > 
   > chunk支持不定长内存的话,第一次申请1624k,free时放回1024k的free list中,在后续当成1024k来用,相比之前一直多浪费400k内存,再次申请1600k时也无法复用上次申请的内存。
   > 
   > 所以我建议 ChunkAllocator 还是保持仅定长内存,否则为了不浪费内存还要加更复杂的比较逻辑,如我上面的patch,对代码健壮性做一些完善,若认可这个思路,可以参考我的patch重新提交下代码~ @muyizi ,若有其他见解欢迎随时沟通~
   
   对应您说的,我认为只支持定长内存其实本质上就不可避免造成内存浪费,用户需要1624k只能去申请2048k,所以我觉得内存浪费这个是一直存在的;
   把allocate不定长的方式不暴露给使用者确实是目前比较简单的方法,但是后面如需使用不定长申请,还是需要改那块逻辑的


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] xinyiZzz commented on pull request #11040: fix_arena_push_size

Posted by GitBox <gi...@apache.org>.
xinyiZzz commented on PR #11040:
URL: https://github.com/apache/doris/pull/11040#issuecomment-1190119739

   @muyizi 
   ChunkAllocator中的chunk大小必须是2^n ,`ChunkAllocator::allocate`不能申请1025,见接口注释,申请1025应该用`ChunkAllocator::allocate_align`
   
   应该在`ChunkAllocator::allocate`中加一个 DCKECK或return error检测size是否为2^n,
   然后把`ChunkAllocator::allocate`变成private,对MemPool设为friend class,对外只暴露`ChunkAllocator::allocate_align`。实际上`ChunkAllocator::allocate`现在只有MemPool使用,MemPool每次申请都是2^n


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] xinyiZzz commented on pull request #11040: fix_arena_push_size

Posted by GitBox <gi...@apache.org>.
xinyiZzz commented on PR #11040:
URL: https://github.com/apache/doris/pull/11040#issuecomment-1200422888

   > > 如果 ChunkAllocator 中的 chunk 支持不定长内存的话,那预期存在内存浪费问题,
   > > 按之前的逻辑,申请1624k会实际申请2048k,free时放回2048k的free list中,在之后满足1024k到2048k的内存申请
   > > chunk支持不定长内存的话,第一次申请1624k,free时放回1024k的free list中,在后续当成1024k来用,相比之前一直多浪费400k内存,再次申请1600k时也无法复用上次申请的内存。
   > > 所以我建议 ChunkAllocator 还是保持仅定长内存,否则为了不浪费内存还要加更复杂的比较逻辑,如我上面的patch,对代码健壮性做一些完善,若认可这个思路,可以参考我的patch重新提交下代码~ @muyizi ,若有其他见解欢迎随时沟通~
   > 
   > 对应您说的,我认为只支持定长内存其实本质上就不可避免造成内存浪费,用户需要1624k只能去申请2048k,所以我觉得内存浪费这个是一直存在的; 把allocate不定长的方式不暴露给使用者确实是目前比较简单的方法,但是后面如需使用不定长申请,还是需要改那块逻辑的
   
   @muyizi  是的,不过上面的commit我在另一个pr顺便修了= =|,对chunk allocator有进一步的优化思路,欢迎继续提交,thks~👏


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] jackwener closed pull request #11040: fix_arena_push_size

Posted by GitBox <gi...@apache.org>.
jackwener closed pull request #11040: fix_arena_push_size
URL: https://github.com/apache/doris/pull/11040


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] xinyiZzz commented on pull request #11040: fix_arena_push_size

Posted by GitBox <gi...@apache.org>.
xinyiZzz commented on PR #11040:
URL: https://github.com/apache/doris/pull/11040#issuecomment-1190267181

   如果 ChunkAllocator 中的 chunk 支持不定长内存的话,那预期存在内存浪费问题,
   
   按之前的逻辑,申请1624k会实际申请2048k,free时放回2048k的free list中,在之后满足1024k到2048k的内存申请
   
   chunk支持不定长内存的话,第一次申请1624k,free时放回1024k的free list中,在后续当成1024k来用,相比之前一直多浪费400k内存,再次申请1600k时也无法复用上次申请的内存。
   
   所以我建议 ChunkAllocator 还是保持仅定长内存,否则为了不浪费内存还要加更复杂的比较逻辑,如我上面的patch,对代码健壮性做一些完善,若认可这个思路,可以参考我的patch重新提交下代码~ @muyizi ,若有其他见解欢迎随时沟通~


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] xinyiZzz commented on pull request #11040: fix_arena_push_size

Posted by GitBox <gi...@apache.org>.
xinyiZzz commented on PR #11040:
URL: https://github.com/apache/doris/pull/11040#issuecomment-1190122477

   refer this
   
   diff --git a/be/src/runtime/memory/chunk_allocator.cpp b/be/src/runtime/memory/chunk_allocator.cpp
   index 6db98ecc9..e69b4c2e8 100644
   --- a/be/src/runtime/memory/chunk_allocator.cpp
   +++ b/be/src/runtime/memory/chunk_allocator.cpp
   @@ -136,6 +136,8 @@ ChunkAllocator::ChunkAllocator(size_t reserve_limit)
    }
   
    Status ChunkAllocator::allocate(size_t size, Chunk* chunk) {
   +    DCHECK(BitUtil::RoundUpToPowerOfTwo(size) == size);
   +
        // fast path: allocate from current core arena
        int core_id = CpuInfo::get_current_core();
        chunk->size = size;
   diff --git a/be/src/runtime/memory/chunk_allocator.h b/be/src/runtime/memory/chunk_allocator.h
   index 43bd88444..fef62e6db 100644
   --- a/be/src/runtime/memory/chunk_allocator.h
   +++ b/be/src/runtime/memory/chunk_allocator.h
   @@ -62,11 +62,7 @@ public:
   
        ChunkAllocator(size_t reserve_limit);
   
   -    // Allocate a Chunk with a power-of-two length "size".
   -    // Return true if success and allocated chunk is saved in "chunk".
   -    // Otherwise return false.
   -    Status allocate(size_t size, Chunk* Chunk);
   -
   +    // Up size to 2^n length, allocate a chunk.
        Status allocate_align(size_t size, Chunk* chunk);
   
        // Free chunk allocated from this allocator
   @@ -78,6 +74,14 @@ public:
        // otherwise the capacity of chunk allocator will be wrong.
        void free(uint8_t* data, size_t size);
   
   +private:
   +    friend class MemPool;
   +
   +    // Allocate a Chunk with a power-of-two length "size".
   +    // Return true if success and allocated chunk is saved in "chunk".
   +    // Otherwise return false.
   +    Status allocate(size_t size, Chunk* Chunk);
   +
    private:
        static ChunkAllocator* _s_instance;


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] zuochunwei commented on pull request #11040: fix_arena_push_size

Posted by GitBox <gi...@apache.org>.
zuochunwei commented on PR #11040:
URL: https://github.com/apache/doris/pull/11040#issuecomment-1190119705

   looks nice


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org