-
Notifications
You must be signed in to change notification settings - Fork 114
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
Refactor for easier deploys of new versions. Make 5.1.0 default #19
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The previous version added leading and trailing colons to these values if a variable had no assigned value before the buildpack script ran.
This allows new versions to be built for uploading by running `make VERSION=5.0.1` rather than updating the hard coded version in multiple places. I've also added support for building against either the legacy cedar-14 architecture or heroku-16. While versions built on cedar-14 appear to run on heroku-16, the newer stack should also mean a newer and better tool chain. I've removed the Dockerfile and switched to using Heroku's docker images directly as this removes the need for cleaning them up afterwards. It possibly makes debugging harder but having build.sh available should avoid some of that. Fixes mojodna#2 Fixes mojodna#7 Fixes mojodna#13
These should only be necessary if someone is planning to link against jemalloc when building other libraries. I'm erring on the side that it's probably unnecessary and I'm removing it to simplify the install command.
It looks like the code was originally taken from a Cairo build pack that had multiple libraries it was installing. Removing it reduces the overall size and complexity of the script.
This is a quality of life addition that makes it easier to configure LD_PRELOAD. It appears the library name changed between 4.x and 5.x which looks to have caused some confusion. It also seems that getting it in the Heroku dashboard acts differently from setting it on the command line (I think).
Allows selecting a precompiled version of jemalloc given a version string. This allows easily switching but requires a rebuild of the slug.
Switching for some testing and until this work is upstreamed.
This reduces the artifact sizes by 20 to 25%.
Until these changes get pulled upstread, make sure anyone who stumbles across this can get it working.
It looks like the server for canonware.com has been decommissioned and all releases are being posted to GitHub now. https://gitter.im/jemalloc/jemalloc?at=59b07988bc46472974040c91
An issue came up that jemalloc 3.6.0 does not include jemalloc-config (it was added in 4.0.0). Researching the issue I discovered that installing jemalloc will create a symlink that points to the correct revision. Using jemalloc-config is more correct especially in cases where multiple versions of jemalloc are installed. Since Heroku dyno's are built from a fresh state on each code change, we can expect that there will only ever be one version of jemalloc installed and the symlink will point to the latest version.
We've seen some great results with this so I'm trying to make getting started with it easier and more understandable. This highlights the configuration options that are available and adds a section for how to deploy new releases (because I'm sure I'll forget in 6 months).
Heroku will be supporting cedar-14 until April 2019, it's cedar-10 that's officially be deprecated. https://devcenter.heroku.com/articles/stack
This removes a redirection to the releases page and gives anyone reading the docs a quick view of what's available. The releases page is still the canonical source and may be updated faster / more often than this list.
When loading profile.d/jemalloc we would get the below error. It boils down to asking bash to do a string comparison when the left side wasn't a string. The fix is to force the left side to be a string. So, if it's unset, the comparison becomes `"" = "true"`. ``` /app/.profile.d/jemalloc.sh: line 5: [: =: unary operator expected ``` Fixes mojodna#1
Mostly for completeness. Most people should be using the newest version available or 3.6.0.
It's currently (2018-04-27) unreleased but Heroku has made the docker base images available. I've cut an initial set of builds against the current image and will do a rebuild once the stack becomes generally available.
There is room for improvement on why you want to use it but I'll flesh that out later.
Jemalloc 5.1.0 was released on 2018-05-08 and is a recommended upgrade by the developers [Release Notes](https://github.com/jemalloc/jemalloc/releases/tag/5.1.0)
The current builds are 7-8MB and I would like to see them get a little smaller. Current tcmalloc builds come in around 1MB. While this doesn't seem to change the numbers much, I prefer that it's explicit rather than whatever tar defaults to.
This reduces the bundle from ~8.2MB for 5.1.0 to 1.13MB which will save bandwidth and slug size. The biggest saving came from removing the static libs which weighed in around 45MB uncompressed. I'm not aware of a usecase where you would prefer the static builds over the shared, so if you're reading this wondering "why were these removed" then feel free to open an issue with your use case. This also removes the include headers. They're really small and compress well but they're only useful when compiling code against jemalloc. I've taken step away from this buildpack being useful for compiling other buildpacks against it, this continues that direction. Lastly, this applies to the builds for 5.1.0 and won't be backported to earlier builds unless there is another reason to rebuild them. I would prefer builds to be stable once uploaded and depended on by folks.
Adding this to avoid extra garbage images from lingering after a build.
After a few months of testing 5.1.0 appears to be stable and working well. It is a recommended upgrade by the jemalloc team. Closes mojodna#2
bf4
commented
Sep 3, 2018
| 4.4.0 | | ||
| 4.5.0 | | ||
| 5.0.1 | | ||
| 5.1.0 | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably shouldn't be merged unless this is acurrate
cc @gaffneyc |
@bf4 See #18 (comment) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
per #18