Have homebrew use system jemalloc and hwloc

[stonea]

Our nightly testing is encountering an issue we're linking to a system version of jemalloc despite configuring Chapel to use a bundled version (ultimately resulting in unsightly linker errors):

Undefined symbols for architecture arm64:
  "_chpl_je_dallocx", referenced from:
      _chpl_free in chpl__module.o
      _chpl_realloc in chpl__module.o
      _chpl_mem_layerInit in libchpl.a(mem-jemalloc.o)
      _qio_readv in libchpl.a(qio.o)
      _chpl_mem_free in libchpl.a(qio.o)
      _qio_writev in libchpl.a(qio.o)
      _qio_preadv in libchpl.a(qio.o)

I believe this is the same issue as: #18955

In order to get nightly testing back on track I think we should use the system version of jemalloc (and hwloc, which I think would run into a similar issue).

[ronawho]

Need depends_on jemalloc too?

[stonea]

It seems like we should add it (it does depend on it after all).

For whatever reason things build fine without it but don't build fine if it's missing hwloc.

[ronawho]

I would guess that's just since jemalloc happens to be installed on our test machine already.

[stonea]

Nah, it would error out even with hwloc installed, here:

chapel/util/chplenv/chpl_hwloc.py

Lines 49 to 53 in 113ce16

	exists, retcode, my_out, my_err = try_run_command(
	['pkg-config', '--atleast-version=2.1', 'hwloc'])
	if exists and retcode != 0:
	err = "CHPL_HWLOC=system requires hwloc >= 2.1"
	error(err, ValueError)

That pkg-config command would work from the root shell but not whatever environment homebrew puts itself in during the install.

[ronawho]

Note that CHPL_HOST_MEM defaults to cstdlib on mac. Not sure we need/want to set it here.

Also don't see any reason to explicitly set CHPL_MEM=jemalloc

[stonea]

I added the CHPL_HOST_MEM line because without it I was running into this (confusing) error message:

Error: CHPL_JEMALLOC must be 'none' when CHPL_MEM is not jemalloc

Confusing because this is despite the fact that I explicitly did set CHPL_MEM to jemalloc.

Taking a look at chpl_jemalloc.get:

chapel/util/chplenv/chpl_jemalloc.py

Line 11 in dbf58e6

def get(flag='target'):

it can be called with its flag parameter as either host or target. If it's host then the mem_val on this line is really referring to CHPL_HOST_MEM and jemalloc_val is really referring to CHPL_HOST_JEMALLOC:

chapel/util/chplenv/chpl_jemalloc.py

Lines 57 to 58 in dbf58e6

	if mem_val != 'jemalloc' and jemalloc_val != 'none':
	error("CHPL_JEMALLOC must be 'none' when CHPL_MEM is not jemalloc")

Maybe we should have the error print CHPL_HOST_JEMALLOC/CHPL_TARGET_JEMALLOC and CHPL_HOST_MEM/CHPL_TARGET_MEM mem as appropriate. Though that might be confusing if the user really did set CHPL_MEM directly and this line in our doc makes me thing maybe it's going to be a moot point in a release or two anyway:

CHPL_TARGET_MEM will be replacing CHPL_MEM in the future. CHPL_TARGET_MEM takes precedence over CHPL_MEM.

In terms of what to do for this homebrew formula: now that I write this out, it seems like a better fix than setting CHPL_HOST_MEM=jemalloc would be to remove the CHPL_HOST_JEMALLOC=system line (in which case I think it defaults to none?). Trying it out this seems to build fine.

Also don't see any reason to explicitly set CHPL_MEM=jemalloc

Yep, looks like that one's not needed.

[ronawho]

In terms of what to do for this homebrew formula: now that I write this out, it seems like a better fix than setting CHPL_HOST_MEM=jemalloc would be to remove the CHPL_HOST_JEMALLOC=system line (in which case I think it defaults to none?). Trying it out this seems to build fine.

👍