Skip to content

Conversation

@SimonSapin
Copy link
Member

@SimonSapin SimonSapin commented Apr 15, 2018

CC https://github.com/alexcrichton/jemallocator/pull/40, rust-lang/rust#49669

The new version of jemallocator requires a more recent jemalloc https://github.com/alexcrichton/jemallocator/pull/34 which doesn’t build on our current Android toolchain jemalloc/jemalloc#1175.

To avoid blocking on figuring that out, duplicate ~70 lines from jemallocator and use the older jemalloc-sys directly.


This change is Reviewable

Cargo.lock Outdated

[[package]]
name = "libc"
version = "0.1.12"
Copy link
Contributor

Choose a reason for hiding this comment

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

Uh ? Could we avoid that duplication ?

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Apr 15, 2018
@SimonSapin SimonSapin force-pushed the alloc branch 2 times, most recently from 2ea085f to 179e0d6 Compare April 15, 2018 21:44
CC https://github.com/alexcrichton/jemallocator/pull/40,
rust-lang/rust#49669

The new version of jemallocator requires a more recent jemalloc
https://github.com/alexcrichton/jemallocator/pull/34
which doesn’t build on our current Android toolchain
jemalloc/jemalloc#1175.

To avoid blocking on figuring that out, duplicate ~70 lines
from jemallocator and use the older jemalloc-sys directly.
@nox
Copy link
Contributor

nox commented Apr 16, 2018

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 7dbc524 has been approved by nox

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Apr 16, 2018
@bors-servo
Copy link
Contributor

⌛ Testing commit 7dbc524 with merge 1c9bbce...

bors-servo pushed a commit that referenced this pull request Apr 16, 2018
Fork the jemallocator crate, fix for nightly-2018-04-15

CC https://github.com/alexcrichton/jemallocator/pull/40, rust-lang/rust#49669

The new version of jemallocator requires a more recent jemalloc https://github.com/alexcrichton/jemallocator/pull/34 which doesn’t build on our current Android toolchain jemalloc/jemalloc#1175.

To avoid blocking on figuring that out, duplicate ~70 lines from jemallocator and use the older jemalloc-sys directly.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20641)
<!-- Reviewable:end -->
pub use super::ffi::{malloc, realloc, free};
}

pub struct Allocator;
Copy link
Member

Choose a reason for hiding this comment

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

Could we get an issue on file to undo this?

Copy link
Member Author

Choose a reason for hiding this comment

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

@bors-servo
Copy link
Contributor

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
Approved by: nox
Pushing 1c9bbce to master...

@bors-servo bors-servo merged commit 7dbc524 into master Apr 16, 2018
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 16, 2018
@SimonSapin SimonSapin deleted the alloc branch April 16, 2018 12:15
@mrobinson mrobinson mentioned this pull request Jan 1, 2024
4 tasks
mrobinson added a commit to mrobinson/servo that referenced this pull request Jan 1, 2024
This dependency was forked in #servo#20641 in order to fix the Android build.
Years have gone by and it's quite likely that many things have changed
in the Android toolchain and these dependencies. We can sort out this
issue when getting the Android build working -- or if all else fails,
disable jemalloc for Android. In the meantime, unfork the dependency and
upgrade it.

Fixes servo#20645.
github-merge-queue bot pushed a commit that referenced this pull request Jan 1, 2024
This dependency was forked in ##20641 in order to fix the Android build.
Years have gone by and it's quite likely that many things have changed
in the Android toolchain and these dependencies. We can sort out this
issue when getting the Android build working -- or if all else fails,
disable jemalloc for Android. In the meantime, unfork the dependency and
upgrade it.

Fixes #20645.
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.

7 participants