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.

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

Add feature disable_cache_oblivious to jemallocator re-exports #104

Merged
merged 2 commits into from
Oct 16, 2024
Merged

Add feature disable_cache_oblivious to jemallocator re-exports #104

merged 2 commits into from
Oct 16, 2024

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 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

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
10 checks passed
@BusyJay
Copy link
Member

BusyJay commented Oct 16, 2024

Thank!

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