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

malloc: optimizations #16037

Merged
merged 2 commits into from
May 11, 2024
Merged

malloc: optimizations #16037

merged 2 commits into from
May 11, 2024

Conversation

reusee
Copy link
Contributor

@reusee reusee commented May 11, 2024

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

issue #15479

What this PR does / why we need it:

malloc: refine cache size calculation

malloc: remove AllocTyped

malloc: remove eviction

malloc: add shard alloc and free statistics

malloc: flush cached objects when idle

malloc: refine cache size calculation

malloc: remove AllocTyped

malloc: remove eviction

malloc: add shard alloc and free statistics

malloc: flush cached objects when idle
@matrix-meow matrix-meow added the size/M Denotes a PR that changes [100,499] lines label May 11, 2024
@matrix-meow
Copy link
Contributor

@reusee Thanks for your contributions!

Pull Request Review:

Title: malloc: optimizations

Body:

  • The PR is categorized as an Improvement.
  • It references issue [Refactoring]: Remove or replace mmap with malloc #15479.
  • The PR aims to optimize the malloc package by refining cache size calculation, removing unnecessary functions like AllocTyped, eviction, adding shard alloc and free statistics, and flushing cached objects when idle.

Changes:

  1. bench_test.go:

    • Removed BenchmarkAllocTyped and BenchmarkParallelAllocTyped functions which are no longer needed.
  2. flush.go:

    • Added a new file flush.go which includes a function to periodically check and flush cached objects when idle.
  3. handle.go:

    • Modified the Free method in the Handle struct to update the number of free objects in the corresponding shard.
  4. malloc.go:

    • Replaced runtime package with sync/atomic.
    • Introduced a new Shard struct to manage allocation statistics.
    • Updated the calculation of bufferedObjectsPerClass and classSumSize.
    • Refactored the initialization of shards to use the new Shard struct.
    • Modified the classAllocate function to update allocation statistics in the corresponding shard.
    • Removed the AllocTyped function as it is no longer used.

Feedback and Suggestions:

  1. Optimization:

    • Consider consolidating the changes related to shard management into a separate function for better organization and readability.
    • Ensure that the changes do not introduce any performance regressions and are thoroughly tested.
  2. Code Quality:

    • Remove commented-out code or unused variables to maintain a clean codebase.
    • Ensure that the code follows consistent naming conventions and formatting throughout the PR.
  3. Security:

    • Review the changes to ensure that there are no potential security vulnerabilities introduced, especially in the new flush.go file.
  4. Documentation:

    • Update the documentation to reflect the changes made in the PR, especially regarding the new functionality added for flushing cached objects.
  5. Testing:

    • Verify that the changes are adequately covered by unit tests to prevent regressions and ensure the reliability of the code.

By addressing the above points, the PR can be further optimized and improved to enhance the overall quality and performance of the malloc package.

@mergify mergify bot merged commit 4d44f72 into matrixorigin:main May 11, 2024
18 of 19 checks passed
reusee added a commit to reusee/matrixone that referenced this pull request May 11, 2024
malloc: refine cache size calculation

malloc: remove AllocTyped

malloc: remove eviction

malloc: add shard alloc and free statistics

malloc: flush cached objects when idle

Approved by: @zhangxu19830126
@matrix-meow matrix-meow mentioned this pull request May 11, 2024
7 tasks
mergify bot pushed a commit that referenced this pull request May 13, 2024
malloc: refine cache size calculation

malloc: remove AllocTyped

malloc: remove eviction

malloc: add shard alloc and free statistics

malloc: flush cached objects when idle

Approved by: @zhangxu19830126, @sukki37
@aylei aylei mentioned this pull request Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement size/M Denotes a PR that changes [100,499] lines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants