forked from jemalloc/jemalloc
-
Notifications
You must be signed in to change notification settings - Fork 6
[WIP] Add windows cmake support. #2
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
Open
Sima214
wants to merge
38
commits into
mattsta:add/cmake-2019
Choose a base branch
from
Sima214:add/cmake-2019/win-patch
base: add/cmake-2019
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or 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
Enable-debug and 64-bit runs tend to be more relevant. Run them first.
`cbopaque` can now be overriden without overriding `write_cb` in the first place. (Otherwise there would be no need to have the `cbopaque` parameter in `malloc_message`.)
The original logic can be disastrous if `PROF_DUMP_BUFSIZE` is less than `slen` -- `prof_dump_buf_end + slen <= PROF_DUMP_BUFSIZE` would always be `false`, so `memcpy` would always try to copy `PROF_DUMP_BUFSIZE - prof_dump_buf_end` chars, which can be dangerous: in the last round of the `while` loop it would not only illegally read the memory beyond `s` (which might not always be disastrous), but it would also illegally overwrite the memory beyond `prof_dump_buf` (which can be pretty disastrous). `slen` probably has never gone beyond `PROF_DUMP_BUFSIZE` so we were just lucky.
`prof_accumbytes` was supposed to be replaced by `prof_accum` in jemalloc#623.
Return a valid pointer instead of failed assertion.
The VirtualAlloc and VirtualFree APIs are different because MEM_DECOMMIT cannot be used across multiple VirtualAlloc regions. To properly support decommit, only allow merge / split within the same region -- this is done by tracking the "is_head" state of extents and not merging cross-region. Add a new state is_head (only relevant for retain && !maps_coalesce), which is true for the first extent in each VirtualAlloc region. Determine if two extents can be merged based on the head state, and use serial numbers for sanity checks.
This can only happen on Windows and with opt.retain disabled (which isn't the default). The solution is suboptimal, however not a common case as retain is the long term plan for all platforms anyway.
extent_register may only fail if the underlying extent and region got stolen / coalesced before we lock. Avoid doing extent_leak (which purges the region) since we don't really own the region.
The counter is 0 unless metadata allocation failed (indicates OOM), and is mainly for sanity checking.
Stop scanning once reached the first max_active_fit size.
When tcache was disabled, the dalloc promoted case was missing.
`prof.c` is growing too long, so trying to modularize it. There are a few internal functions that had to be exposed but I think it is a fair trade-off.
Refactored core profiling codebase into two logical parts: (a) `prof_data.c`: core internal data structure managing & dumping; (b) `prof.c`: mutexes & outward-facing APIs. Some internal functions had to be exposed out, but there are not that many of them if the modularization is (hopefully) clean enough.
This reverts commit 0b46240.
This reverts commit 7618b0b.
W/o retain, split and merge are disallowed on Windows. Avoid doing first-fit which needs splitting almost always. Instead, try exact fit only and bail out early.
g++ 5.5.0+ complained `parameter ‘expected’ set but not used [-Werror=unused-but-set-parameter]` (despite that `expected` is in fact used).
The emitter APIs used were incorrect, a side effect of which was extra lines being printed.
This implements CMake infrastrucutre in preparation for jemalloc 5.2.0
CMake build supports features from configure.ac:
- automatic symbol extraction (je_ and jet_)
- auto-generate awk formatting files
- build all objects with -DJEMALLOC_NO_PRIVATE_NAMESPACE
- run nm -a against object files and pipe through awk
- output private header files
- platform feature detection
- tests adapted from configure.ac into CMake check_c_source_runs()
- header generation from .h.in to .h based on config-discovered features
- 4-way building
- generate je_ objects with symbols exposed
- generate je_ objects for building
- generate jet_ objects with symbols exposed
- generate jet_ objects for building
- make test (currently only exposed when CMake release is Debug)
- C++14 wrapper built and included in library by default
- this means CMake recognizes libjemalloc.{a,so,dylib} as a C++ library
and any projects using it will be linked with C++
- the configure option JEMALLOC_CXX_DISABLE can disable including the
C++ wrapper which will cause CMake to see jemalloc as a C-only
library again
- per-OS feature flags
- user configuration options (the old ./configure --[opt] settings)
- many options implemented, but not all yet
- and the implemented options haven't all been tested either
- options can be edited after configuration with `make edit_cache`
- but may require a clean build/ directory because we aren't
regenerating feature headers on each post-config-config update
Featues unique to this CMake build infrastructure:
- complete out-of-source builds
- all headers and supporting artifacts are generated in build/
- mkdir build; cd build; cmake ..; make -j4
- no "make distclean" required to remove build cruft
- just `rm -rf build/` and you have a 100% clean checkout again
- better build system organization:
- compiler detection: build-aux/DetectCompilerFeatures.cmake
- OS detection: build-aux/DetectOSFeatures.cmake
- exposed config options: build-aux/UserCompileOptions.cmake
Features ignored or not complete right now:
- installing
- doc building
- shell wrapper creation (bin/jemalloc.sh, etc)
- dual building of pic and non-pic objects/archives
- windows build support
- proper library-wide mangle of override functions
- refactoring the original jemalloc-cmake approach to tests
- it works currently, but the CMake directives are convoluted in places
History:
This original CMake conversion started by using CMake files from
jemalloc/jemalloc-cmake, but (for some reason?) the jemalloc-cmake
repository is actually a windows-only CMake port. Also, the git
history there is so outdated and polluted we can't use commits from it
against the live jemalloc/jemalloc repository.
Those intial CMake conversion scripts were a useful starting point though.
For re-CMaking inspiration, I started with the CMake files from:
jemalloc/jemalloc-cmake as of
88c2dc8
Thu Dec 15 16:26:43 2016 -0800
and fixed it to build on non-windows platforms (while removing some of
the windows-only operations and other irrelevant behaviors since they
can't easily be tested (or even run?) on other platforms).
The initial jemalloc-cmake infrastructure had a lot of useful utilities
for building CMake, translating autoconf includes, and finding system
properties. I kept most of those in place and they saved much
trial-and-error shuffling around trying to get output text files
formatted properly.
If Windows support is desired again, support should be added to this
CMake deployment revision instead of the abandoned jemalloc/jemalloc-cmake.
When merging, this commit should be squashed into the previous commit since they all form one logical commit: git rebase -i --autosquash origin/dev This commit contains only fixes to the previous commit.
Multi-purpose bulk commit to rebase into the final merge: - greatly improves proper rebuilding of generated files based on when their underlying dependencies or assumptions (.git modifications) change - improves reliability of private header generation - attempted (and failed, but left notes about) moving symbol extraction from an one-for-all approach to a one-for-one approach, but CMake is fighting us on a clean solution. (the simplest solution at this point would be to disable the COMMENT blocks on the custom symbol and header generation commands so they don't clutter output and can run whenever they need to run) - removed some other legacy jemalloc cmake attempts from years ago - improved private namespace user config options - various refactoring, logic cleanup, and more explicit comments
Improve test target names so they don't conflict when being included in other projects (generic names like 'stats' and 'zero' have a high chance of conflicting with other projects). Also improves overall test organization (direct names everywhere instead of unnecessarily templated variables) plus test creation through reusing a function instead of having copy/pasted test creation in five directories.
Adds CMake options to enable/disable STATIC and SHARED libraries as a user preference. Also fixes some dependency logic around awk creation.
Add new test broken out from unit/mallctl.c into unit/extent_util.c
Cleanup some style issues and a bad copy/paste comment from previously splitting out functionality into two files.
0005f21 to
0cdaff1
Compare
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.
Configuration:
Win10 64bit with mingw, cmake, gawk, git.
TODO: