{"action":"created","issue":{"url":"https://api.github.com/repos/chapel-lang/chapel/issues/6133","repository_url":"https://api.github.com/repos/chapel-lang/chapel","labels_url":"https://api.github.com/repos/chapel-lang/chapel/issues/6133/labels{/name}","comments_url":"https://api.github.com/repos/chapel-lang/chapel/issues/6133/comments","events_url":"https://api.github.com/repos/chapel-lang/chapel/issues/6133/events","html_url":"https://github.com/chapel-lang/chapel/pull/6133","id":225116395,"number":6133,"title":"Enable use of system jemalloc","user":{"login":"mppf","id":3653132,"avatar_url":"https://avatars0.githubusercontent.com/u/3653132?v=3","gravatar_id":"","url":"https://api.github.com/users/mppf","html_url":"https://github.com/mppf","followers_url":"https://api.github.com/users/mppf/followers","following_url":"https://api.github.com/users/mppf/following{/other_user}","gists_url":"https://api.github.com/users/mppf/gists{/gist_id}","starred_url":"https://api.github.com/users/mppf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mppf/subscriptions","organizations_url":"https://api.github.com/users/mppf/orgs","repos_url":"https://api.github.com/users/mppf/repos","events_url":"https://api.github.com/users/mppf/events{/privacy}","received_events_url":"https://api.github.com/users/mppf/received_events","type":"User","site_admin":false},"labels":[{"id":514878968,"url":"https://api.github.com/repos/chapel-lang/chapel/labels/stat:%20Needs%20Code%20Review","name":"stat: Needs Code Review","color":"c5def5","default":false},{"id":514878918,"url":"https://api.github.com/repos/chapel-lang/chapel/labels/stat:%20Needs%20Design%20Review","name":"stat: Needs Design Review","color":"c5def5","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":6,"created_at":"2017-04-28T15:33:07Z","updated_at":"2017-05-11T18:28:09Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/chapel-lang/chapel/pulls/6133","html_url":"https://github.com/chapel-lang/chapel/pull/6133","diff_url":"https://github.com/chapel-lang/chapel/pull/6133.diff","patch_url":"https://github.com/chapel-lang/chapel/pull/6133.patch"},"body":"This PR adds support for using a system jemalloc library. This are intended to support the Debian packaging effort, but is useful more generally.\r\n\r\nIt enables:\r\n * CHPL_HWLOC=system\r\n * CHPL_MEM=jemalloc CHPL_JEMALLOC=system\r\n\r\nDetails:\r\n\r\n * Originally, this PR added CHPL_MEM=jemalloc-system. But that is problematic because it both specifies the memory layer to use (jemalloc) and also specifies where to get it.\r\n  \r\n   Instead of doing that, add a CHPL_JEMALLOC variable that is only useful when CHPL_MEM=jemalloc. That way, each hase one meaning:\r\n     * CHPL_MEM=jemalloc -> use jemalloc\r\n     * CHPL_JEMALLOC -> where to get jemalloc\r\n\r\n   Adding a CHPL_JEMALLOC variable involved making these changes:\r\n       - adding util/chplenv/chpl_3p_jemalloc_configs.py\r\n       - calling get_link_args from util/chplenv/chpl_3p_jemalloc_configs.py in printchlenv\r\n       - makefiles:\r\n            - runtime/etc/Makefile.jemalloc-`$CHPL_JEMALLOC`\r\n            - call those from runtime/etc/Makefile.include\r\n            - runtime/make/Makefile.runtime.jemalloc-`$CHPL_JEMALLOC`\r\n            - call those from runtime/make/Makefile.runtime.include\r\n\r\n * To work with a system-installed jemalloc, besides adding Makefiles and scripting to support CHPL_MEM=jemalloc-system, I needed to update some of our C code working with jemalloc in chpl-mem-impl.h and mem-jemalloc.c:\r\n\r\n   * update our calls to jemalloc routines to use a configurable prefix (currently chpl_je_ or je_)\r\n   * use #defines to make it compile with jemalloc 3.6\r\n     *  3.6 is the default on Ubuntu 17.04 and Debian sid (so this version is important for the Debian packaging effort)\r\n     * 3.6 doesn't include chunk_hooks_t and doesn't define `bool`\r\n     * we use chunk_hooks_t for multilocale + segment=fast\r\n     * I made it compile but fail at runtime in a multilocale + segment=fast configuration\r\n   * use -I to the directory containing jemalloc/jemalloc.h since the system install has /usr/include/jemalloc/jemalloc.h - that is, we now include \"jemalloc/jemalloc.h\" instead of just \"jemalloc.h\"\r\n\r\n(note, system packages were tested with Ubuntu 17.04 packages)\r\n\r\n`make clobber && make && make check` with:\r\n- [ ] quickstart config\r\n- [ ] quickstart config + CHPL_MEM=jemalloc\r\n- [ ] quickstart config + CHPL_MEM=jemalloc CHPL_JEMALLOC=system\r\n\r\nVerified that quickstart + CHPL_MEM=jemalloc CHPL_JEMALLOC=system does not build anything in third-party.\r\n\r\nVerified an error whet CHPL_JEMALLOC=system but system jemalloc is not installed\r\n (`fatal error: jemalloc/jemalloc.h: No such file or directory`)\r\n\r\nTODOs\r\n- [ ] full local testing\r\n- [ ] check with Ben A\r\n- [ ] make CHPL_JEMALLOC appear as sub-setting to CHPL_MEM"},"comment":{"url":"https://api.github.com/repos/chapel-lang/chapel/issues/comments/300877045","html_url":"https://github.com/chapel-lang/chapel/pull/6133#issuecomment-300877045","issue_url":"https://api.github.com/repos/chapel-lang/chapel/issues/6133","id":300877045,"user":{"login":"ben-albrecht","id":3595449,"avatar_url":"https://avatars2.githubusercontent.com/u/3595449?v=3","gravatar_id":"","url":"https://api.github.com/users/ben-albrecht","html_url":"https://github.com/ben-albrecht","followers_url":"https://api.github.com/users/ben-albrecht/followers","following_url":"https://api.github.com/users/ben-albrecht/following{/other_user}","gists_url":"https://api.github.com/users/ben-albrecht/gists{/gist_id}","starred_url":"https://api.github.com/users/ben-albrecht/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ben-albrecht/subscriptions","organizations_url":"https://api.github.com/users/ben-albrecht/orgs","repos_url":"https://api.github.com/users/ben-albrecht/repos","events_url":"https://api.github.com/users/ben-albrecht/events{/privacy}","received_events_url":"https://api.github.com/users/ben-albrecht/received_events","type":"User","site_admin":false},"created_at":"2017-05-11T18:28:09Z","updated_at":"2017-05-11T18:28:09Z","body":"Just want to comment on 2 design decisions that have been made:\r\n\r\n#### `CHPL_JEMALLOC=system` vs. `CHPL_MEM=jemalloc-system`\r\n\r\nMy initial reaction was that `CHPL_JEMALLOC=system` is going to generate a ton of new env vars to manage/document as we support more system-installed third-parties. However, I think it's reasonable to expect that we'll only support 1 system-installed variant per configuration, e.g. `CHPL_MEM`, and so it shouldn't get too out of hand. There is also a benefit in developer and user code for not having to check the value of `CHPL_MEM` as `jemalloc*`.\r\n\r\n#### public vs. internal\r\n\r\nGiven the difficulties of ensuring that a system-installed jemalloc will work for a given setup, and the limited usefulness of doing so, I would be in favor of making this feature unavailable and/or undocumented to users. It really only benefits the packaging use-case."}}
{"actor":{"display_login":"ben-albrecht"}}