Skip to content

mimalloc followup (not for upstream) #37304

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.

Sign up for GitHub

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

Jump to bottom
Closed
azat wants to merge 26 commits into from
Closed

mimalloc followup (not for upstream) #37304

azat wants to merge 26 commits into from

Conversation

@azat
Copy link
Member

@azat azat commented May 17, 2022 •
edited
Loading

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

This is a help PR for #37159, that includes:

l1tsolaiki and others added 26 commits May 13, 2022 19:13
@l1tsolaiki
Rebase onto master
ec49701
@l1tsolaiki
Fix typo in server cmake + use mimalloc functions in memory.h
af61a0c
@l1tsolaiki
Rebase onto master
1ffbf7b
@l1tsolaiki
Fix build of mimalloc
b3e652a
@l1tsolaiki
Minor refactor to launch check again
95ea5a9
@l1tsolaiki
Update
356c7f0
@l1tsolaiki
Fix typo
1445cc1
@l1tsolaiki
On -> ON, Off -> OFF in mimalloc and jemalloc cmake
3c00d35
@l1tsolaiki
Add flag to docker packager
7205b3c
@l1tsolaiki
Add flag to fasttest docker
a9638ab
@l1tsolaiki
malloc_usable_size -> mi_malloc_usable_size
78ef956
@l1tsolaiki
remove mi_malloc_usable_size for now
8537c52
@l1tsolaiki
Try to override malloc in clickhouse_malloc.cpp
f0360a5
@l1tsolaiki
Try again with commented out mi_malloc_usable size + add more memory …
e50c589 
…quota to previously failing

test
@l1tsolaiki
Increase quota even more
82ac5bc
@l1tsolaiki
Increase qouta in one more query
bd0f254
@l1tsolaiki
Add mimmalloc-override.h instead of mimalloc.h, so had to rename free…
ccc46bf 
… in CurrentMemoryTracker
@l1tsolaiki
Remove include of cstdlib in clickhouse_malloc.cpp
2d499f4
@l1tsolaiki
Comment out include of malloc.h in memory.h
6f786bb
@azat
Merge remote-tracking branch 'upstream/master' into mimalloc
c097192
@azat
mimalloc: fix cmake warning
42e887b 
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
@azat
mimalloc: fix alignment in cmake
3f1f37c 
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
@azat
mimalloc: set MI_MALLOC_OVERRIDE to override free/malloc/...
8a0a6b2 
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
@azat
mimalloc: patch alloc-override.c to not override new/delete for Click…
21fa9f2 
…House

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
@azat
mimalloc: mimalloc-override.h should not be used
d234dcc 
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
@azat
mimalloc: bump mimalloc to fix aligned_alloc()
bd988d4 
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
@azat azat mentioned this pull request May 17, 2022
Closed
@azat azat changed the title mimalloc followup mimalloc followup (do not merge) May 17, 2022
@robot-ch-test-poll1 robot-ch-test-poll1 added pr-not-for-changelog This PR should not be mentioned in the changelog submodule changed At least one submodule changed in this PR. labels May 17, 2022
@azat azat changed the title mimalloc followup (do not merge) mimalloc followup (not for upstream) May 17, 2022
@azat
Copy link
Member Author

azat commented May 18, 2022

Stateless tests (debug, actions) [1/3] — Tests are not finished, fail: 1, passed: 249, skipped: 1

2022.05.18 02:59:34.538420 [ 8612 ] {} <Fatal> BaseDaemon: (version 22.5.1.1, build id: FDE6EA175914E04C) (from thread 5148) (query_id: b91f7886-657c-4bda-b59a-4f5a89cf529f) (query: SELECT test_table.a FROM test_table ORDER BY (test_table.a > test_table.b) + 1 AND (test_table.a > test_table.c) + 1;) Received signal Segmentation fault (11)
2022.05.18 02:59:34.538556 [ 8612 ] {} <Fatal> BaseDaemon: Address: 0x20878240000 Access: read. Attempted access has violated the permissions assigned to the memory area.

@alexey-milovidov
Copy link
Member

alexey-milovidov commented Jul 30, 2022

@azat What does it mean - did we find a bug in mimalloc?

@azat
Copy link
Member Author

azat commented Jul 31, 2022 •
edited
Loading

What does it mean - did we find a bug in mimalloc?

Yep, small problem with alignment (see referenced PRs) and also avoid overriding new/delete for C++ (since clickhouse does this by it's own)

And let's close this for now, since I doubt that mimalloc will be better and seems that the original author postponed this.

@azat azat closed this Jul 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

pr-not-for-changelog This PR should not be mentioned in the changelog submodule changed At least one submodule changed in this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@azat @alexey-milovidov @robot-ch-test-poll1 @l1tsolaiki