`mimalloc-override.o` or `mimalloc.o`? about mimalloc HOT 6 CLOSED

I am kinda convinced by @solvingj that adding different names based on the build options will be too much. It is probably better to stick to the most used common defaults; That is, for the make install:

• libmimalloc.so/dll/dylib: dynamic link library: always use override so it works with LD_PRELOAD out of the box.
• mimalloc.o/obj: static object file: always use override as it is meant especially for use in statically overriding in an existing program.
• mimalloc.lib: static library: here it actually makes sense then to not use override by default and use this when static linking against the mi_ malloc API (perhaps using a header file to redirect).

The debug version is always postfixed with -debug. The only change then is to not use override for the static library by default but otherwise things stay the same -- perhaps with specific options for overriding per build target.

from mimalloc.

Ah, thanks for spotting this. Currently it generates indeed mimalloc.o (and that is the right one that will override by default).

It brings up a bit of an issue -- currently the basename can be mimalloc, mimalloc-secure, and mimalloc-debug. But each can override standard malloc/free depending on the MI_OVERRIDE setting which is ON be default.

Naming wise, we could either

1. always add a -override postfix to the basename. This would be clear, but break existing build scripts as we change the names. Or,
2. add a -no-override (or something?) if the build was with overriding off. This maintains compatibility with the current situation. Also, a build like this is less common so perhaps it is the one that should get a less short name.

I need to think a bit more about this. For now you can use mimalloc.o though

from mimalloc.

Adding -no-override would break some of my scripts

from mimalloc.

I see. But I would argue that the current approach is certainly not ideal as we name both the variants (override and no-override) the same way. So, one of the variants should get a postfix and some buildscripts will break. No easy way around this.

from mimalloc.

I submit that the premise of trying to encode some subset of the build/compiler options in the filename of the output artifact is causing more difficulty than it is solving, and is not generally a good practice for related reasons. It probably made sense for flexibility while tinkering around with different options in a vacuum during the initial development of this library. Perhaps you want to test a few of the artifacts side-by-side in the same folder.

However, when moving to the stage of publishing a library for consumption by a wide audience with a wide range of use-cases, any sort of non-standard customization related to building and linking becomes an issue very quickly. This library has been out for only a few weeks, and already it's in an awkward situation and worried about surrounding breaking existing consumers. While trying to add more custom flexibility for more cases, it dramatically complicates the consumption experience for the simple cases.

I think this library is a rare case where offering the option to expose the object file directly (in addition to only providing a traditional static lib) makes sense and is worth the extra complexity. However, building conditions around the filename based on some concatenation of variables is just too much.

An alternative (and more common) approach would be to always output mimalloc.o, use different output directories to deal with letting multiple variants exist on-disk at the same time. This is part of the reason why linkers take a list of "lib search directories", and a list of "lib names". It's far more common for libraries to output artifacts using a single consistent name, and define custom output directories. Because it's more common, it's easier for everyone (and other tools) to manage and understand.

With that said, I'll understand if the decision is made to keep working with this custom lib naming scheme, I just wanted to share the perspective.

from mimalloc.

@daanx if this change is really necessary I think we should do it in the shortest possible time, waiting too much we risk to make the breaking of already existing scripts/packages much more annoying

from mimalloc.