Skip to content

Add feature disable_cache_oblivious to jemallocator re-exports#104

Merged
BusyJay merged 2 commits intotikv:mainfrom
miried:main
Oct 16, 2024
Merged

Add feature disable_cache_oblivious to jemallocator re-exports#104
BusyJay merged 2 commits intotikv:mainfrom
miried:main

Conversation

@miried
Copy link

@miried miried commented Oct 15, 2024

As described in #103, this feature could not be enabled.

I also updated the README.md to describe that there are more features than only the re-exported ones. Le me know if I should add anything else.

@ti-chi-bot
Copy link

ti-chi-bot bot commented Oct 15, 2024

Welcome @miried! It looks like this is your first PR to tikv/jemallocator 🎉

BusyJay
BusyJay reviewed Oct 15, 2024
View reviewed changes
Copy link
Member

@BusyJay BusyJay left a comment

Choose a reason for hiding this comment

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

Rest LGTM

jemallocator/README.md Outdated

* `alloc_trait` When the `alloc_trait` feature of this crate is enabled, it also implements the `Alloc` trait, allowing usage in collections.

* `default` feature is `background_threads_runtime_support`
Copy link
Member

@BusyJay BusyJay Oct 15, 2024

Choose a reason for hiding this comment

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

Missing a trailing ..

@BusyJay
Copy link
Member

BusyJay commented Oct 15, 2024

Can you also help fix clippy and docs warnings?

@miried
Copy link
Author

miried commented Oct 15, 2024 •
edited
Loading

I'm having trouble reproducing this locally. cargo build fails on my machine with

  --- stderr
  configure: error: cannot find sources (Makefile.in) in /Users/user/code/jemallocator/target/debug/build/tikv-jemalloc-sys-26cea8cce1562f9e/out/build or ..
  tail: /Users/user/code/jemallocator/target/debug/build/tikv-jemalloc-sys-26cea8cce1562f9e/out/build/config.log: No such file or directory
  thread 'main' panicked at jemalloc-sys/build.rs:388:9:

there seem to be 2 competing folders tikv-jemalloc-sys-* but I'm not sure what's causing it.

As for the clippy & docs, was that caused by my patch? or should this be addressed in a separate PR?

@BusyJay
Copy link
Member

BusyJay commented Oct 15, 2024

The error is probably due to #32.

As for the clippy & docs, was that caused by my patch? or should this be addressed in a separate PR?

Unlikely caused by your patch. It may just due to rustc evolution itself. Addressed in this PR or a separate PR are both OK, but we need to get all CI green before merging.

@miried
Copy link
Author

miried commented Oct 15, 2024

that helped. see #105 for patch

@miried
Copy link
Author

miried commented Oct 16, 2024

I rebased the patches on top of the patches. Now I think it will be all green.

@BusyJay BusyJay merged commit fa31efd into tikv:main Oct 16, 2024
@BusyJay
Copy link
Member

BusyJay commented Oct 16, 2024

Thank!

x86pup pushed a commit to x86pup/jemallocator that referenced this pull request Oct 23, 2024
@miried @x86pup
Add feature disable_cache_oblivious to jemallocator re-exports (tik…
d87938b 
…v#104)

Signed-off-by: Michael Rieder <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BusyJay BusyJay BusyJay approved these changes

Assignees

No one assigned

Labels

dco-signoff: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@miried @BusyJay