Skip to content

chore: remove unused jemalloc configs - #780

Open
topocount wants to merge 3 commits into
mainfrom
kjs/mempool-fixes
Open

chore: remove unused jemalloc configs#780
topocount wants to merge 3 commits into
mainfrom
kjs/mempool-fixes

Conversation

@topocount

Copy link
Copy Markdown
Contributor

These configs were not exported and were therefore unused. removing them since they don't seem to be needed.

If needed in the future, export them as _rjem_malloc_conf

These configs were not exported and were therefore unused. removing them since they don't seem to be needed.

If needed in the future, export them as _rjem_malloc_conf
@vercel

vercel Bot commented Apr 1, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
snapchain-docs Ready Ready Preview Aug 6, 2026 1:53pm

Request Review

manan19 added a commit that referenced this pull request Apr 24, 2026
Per PR review (#796), the original commit removed too much. jemalloc
remains the global allocator and the tikv-jemallocator / tikv-jemalloc-ctl
deps + rocksdb \"jemalloc\" feature stay in place. Only the malloc_conf
export — which was unused as topocount noted in the original #780
proposal — is removed. emit_jemalloc_stats and its callers are
restored.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@manan19
manan19 requested a lite review from Copilot April 26, 2026 15:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR removes a jemalloc configuration export from the crate root, keeping the jemalloc allocator enabled (non-MSVC targets) while dropping a configuration string that was believed to be ineffective/unneeded.

Changes:

  • Removed the exported malloc_conf static from src/lib.rs, leaving only the global allocator setup.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/lib.rs
Comment on lines 1 to 3
#[cfg(not(target_env = "msvc"))]
#[global_allocator]
static GLOBAL: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc;
@github-actions

Copy link
Copy Markdown

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

No lines with coverage information in this diff.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/lib.rs:3

  • PR description says the jemalloc configs "were not exported". However, the removed block used #[export_name = "malloc_conf"] on a pub static, which does export the malloc_conf symbol at link time. This change therefore removes an (effective) jemalloc runtime configuration and reverts to jemalloc defaults. Please update the PR description to reflect the actual behavior change (or, if the intent is still to configure jemalloc, export the correct symbol name expected by tikv_jemallocator).
#[cfg(not(target_env = "msvc"))]
#[global_allocator]
static GLOBAL: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants