Build Failed on alpine:3.14: "multiple definition of `RedisModule_Alloc'; " redismodule.h:115: first defined here

[yorkane]

Appreciate great job! I've tried to make alpine-docker-image, but failed. Looking forward your help!

Thanks,

Build logs as below:

[ 45%] Linking C shared library libredis-roaring.so
/usr/bin/cmake -E cmake_link_script CMakeFiles/redis-roaring.dir/link.txt --verbose=1
/usr/bin/cc -fPIC  -std=gnu99 -Wall -Werror -g -shared -Wl,-soname,libredis-roaring.so -o libredis-roaring.so CMakeFiles/redis-roaring.dir/src/redis-roaring.c.o CMakeFiles/redis-roaring.dir/src/type.c.o CMakeFiles/redis-roaring.dir/src/data-structure.c.o CMakeFiles/redis-roaring.dir/deps/sds/sds.c.o   -L/code/redis/redis-roaring/redis-roaring/deps/hiredis
/usr/lib/gcc/x86_64-alpine-linux-musl/10.3.1/../../../../x86_64-alpine-linux-musl/bin/ld: CMakeFiles/redis-roaring.dir/src/type.c.o:/code/redis/redis-roaring/redis-roaring/src/redismodule.h:115: multiple definition of `RedisModule_Alloc'; CMakeFiles/redis-roaring.dir/src/redis-roaring.c.o:/code/redis/redis-roaring/redis-roaring/src/redismodule.h:115: first defined here
/usr/lib/gcc/x86_64-alpine-linux-musl/10.3.1/../../../../x86_64-alpine-linux-musl/bin/ld: CMakeFiles/redis-roaring.dir/src/type.c.o:/code/redis/redis-roaring/redis-roaring/src/redismodule.h:116: multiple definition of `RedisModule_Realloc'; CMakeFiles/redis-roaring.dir/src/redis-roaring.c.o:/code/redis/redis-roaring/redis-roaring/src/redismodule.h:116: first defined here
/usr/lib/gcc/x86_64-alpine-linux-musl/10.3.1/../../../../x86_64-alpine-linux-musl/bin/ld: CMakeFiles/redis-roaring.dir/src/type.c.o:/code/redis/redis-roaring/redis-roaring/src/redismodule.h:117: multiple definition of `RedisModule_Free'; CMakeFiles/redis-roaring.dir/src/redis-roaring.c.o:/code/redis/redis-roaring/redis-roaring/src/redismodule.h:117: first defined here
/usr/lib/gcc/x86_64-alpine-linux-musl/10.3.1/../../../../x86_64-alpine-linux-musl/bin/ld: CMakeFiles/redis-roaring.dir/src/type.c.o:/code/redis/redis-roaring/redis-roaring/src/redismodule.h:118: multiple definition of `RedisModule_Calloc'; CMakeFiles/redis-roaring.dir/src/redis-roaring.c.o:/code/redis/redis-roaring/redis-roaring/src/redismodule.h:118: first defined here
/usr/lib/gcc/x86_64-alpine-linux-musl/10.3.1/../../../../x86_64-alpine-linux-musl/bin/ld: CMakeFiles/redis-roaring.dir/src/type.c.o:/code/redis/redis-roaring/redis-roaring/src/redismodule.h:119: multiple definition of `RedisModule_Strdup'; CMakeFiles/redis-roaring.dir/src/redis-roaring.c.o:/code/redis/redis-roaring/redis-roaring/src/redismodule.h:119: first defined here
/usr/lib/gcc/x86_64-alpine-linux-musl/10.3.1/../../../../x86_64-alpine-linux-musl/bin/ld: CMakeFiles/redis-roaring.dir/src/type.c.o:/code/redis/redis-roaring/redis-roaring/src/redismodule.h:120: multiple definition of `RedisModule_GetApi'; CMakeFiles/redis-roaring.dir/src/redis-roaring.c.o:/code/redis/redis-roaring/redis-roaring/src/redismodule.h:120: first defined here
...

collect2: error: ld returned 1 exit status
make[2]: *** [CMakeFiles/redis-roaring.dir/build.make:148: libredis-roaring.so] Error 1
make[2]: Leaving directory '/code/redis/redis-roaring/redis-roaring/build'
make[1]: *** [CMakeFiles/Makefile2:90: CMakeFiles/redis-roaring.dir/all] Error 2

[aviggiano]

Hi there

Thank you for reporting this issue. I'll take a look at the Docker build soon.

[yorkane]

Hi, I had done some changes for easier inergration, and removed cmake framework. I've uploaded source code there: https://github.com/yorkane/redis-roaring

Sorry for no PR

[aviggiano]

Hey sorry for the late reply and thanks for the update. I hven't had time to take a look at this yet

I'll try to integrate your changes to the main repo.

Cmake is harder to grasp but it provides some benefits in terms of supporting multiple platforms and builds, IMO, so I think I'll try to adapt what you did here.

I'll keep you posted

[yorkane]

Thanks for the update! Seems the following code not work. The roaring_bitmap_size_in_bytes method always return 16 Any ideas?

void BitmapRdbSave(RedisModuleIO *rdb, void *value) {
  Bitmap *bitmap = value;
  size_t serialized_max_size = roaring_bitmap_size_in_bytes(bitmap);
  char *serialized_bitmap = RedisModule_Alloc(serialized_max_size);
  size_t serialized_size = roaring_bitmap_serialize(bitmap, serialized_bitmap);
  RedisModule_SaveStringBuffer(rdb, serialized_bitmap, serialized_size);
  RedisModule_Free(serialized_bitmap);
}

[aviggiano]

Well this looks like a bug. Although you could be looking at really small bitmaps. Do you have a reproducible example I can try?

[aviggiano]

hi @yorkane

I fixed the docker build. It seems that redismodule.h have definitions as well as declarations, which was causing duplicate declaration errors, so I removed one of the files that exported it in addition to the main redis-roaring C file. Not an ideal solution, but at least it works for the moment.

Now, regarding the bitmap size, there are integration_1.sh and integration_2.sh files that check for SAVE and LOAD redis commands, but not for the key size. Maybe that's not specific enough

[yorkane]

Thanks for the quick response! But I can't find integration_1.sh and integration_1.sh in the latest commit.

Here the test code:

/**
 * R.ExportBin <key>
 * */
int RExportBinCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
  if (argc != 2) {
    return RedisModule_WrongArity(ctx);
  }
  // Detect Key is exsit
  RedisModuleKey *key = RedisModule_OpenKey(ctx, argv[1], REDISMODULE_READ);
  int type = RedisModule_KeyType(key);
  if (type == REDISMODULE_KEYTYPE_EMPTY) {
    return RedisModule_ReplyWithError(ctx, "bitmap key not exist");
  }

  // Detect value is bitmap type
  if (RedisModule_ModuleTypeGetType(key) != BitmapType) {
    return RedisModule_ReplyWithError(ctx, REDISMODULE_ERRORMSG_WRONGTYPE);
  }

  RedisModule_AutoMemory(ctx);

  Bitmap* bitmap = RedisModule_ModuleTypeGetValue(key);

  size_t serialized_max_size = roaring_bitmap_size_in_bytes(bitmap);
  char *serialized_bitmap = RedisModule_Alloc(serialized_max_size);
  size_t serialized_size = roaring_bitmap_serialize(bitmap, serialized_bitmap);

  // Redis will handle free-memroy phase
  //RedisModule_ReplyWithStringBuffer(ctx, serialized_bitmap, serialized_size);

// for debug
 RedisModule_ReplyWithLongLong(ctx, serialized_size );
  return REDISMODULE_OK;
}

 if (RedisModule_CreateCommand(ctx, "R.ExportBin", RExportBinCommand, "readonly", 1, 1, 1) == REDISMODULE_ERR) {
    return REDISMODULE_ERR;
  }

$ redis-cli
# create a roaring bitmap with numbers from 1 to 999
127.0.0.1:6379> R.SETRANGE test 1 1000

# get all the numbers as an integer array
127.0.0.1:6379> R.EXPORTBIN test
# output 16, but correct size is 2016

[aviggiano]

@yorkane sorry, I meant here

Why is 2016 the correct size of test? I would expect creating a bitmap full of set bits to be extremely condensed/optimized, and its size to grow only after the sparsity of the bit set increases.

Indeed, I have found this assumption to be correct after testing out the MWE from the CRoaring github repo.

The R.SETRANGE redis command uses roaring_bitmap_from_range under the hood, which is more efficient than for (...) roaring_bitmap_add(...), which is what R.SETBIT uses. So if you perform a batch of R.SETBIT you end up consuming more memory than using R.SETRANGE.

Here's the demo using CRoaring for your test case: https://github.com/aviggiano/croaring-size-demo

$ git clone https://github.com/aviggiano/croaring-size-demo && cd croaring-size-demo
$ cmake -B build . && cmake --build build && ./build/reproc

CMake Deprecation Warning at build/_deps/roaring-src/CMakeLists.txt:1 (cmake_minimum_required):
  Compatibility with CMake < 2.8.12 will be removed from a future version of
  CMake.

  Update the VERSION argument <min> value or use a ...<max> suffix to tell
  CMake that the project does not need compatibility with older versions.

CMake Warning (dev) at build/_deps/roaring-src/CMakeLists.txt:10 (project):
  Policy CMP0048 is not set: project() command manages VERSION variables.
  Run "cmake --help-policy CMP0048" for policy details.  Use the cmake_policy
  command to set the policy and suppress this warning.

  The following variable(s) would be set to empty:

    PROJECT_VERSION
    PROJECT_VERSION_MAJOR
    PROJECT_VERSION_MINOR
    PROJECT_VERSION_PATCH
This warning is for project developers.  Use -Wno-dev to suppress it.

-- BENCHMARK_DATA_DIR: /Users/antonioguilhermeferreiraviggiano/Projects/aviggiano/croaring-size-demo/build/_deps/roaring-src/benchmarks/realdata/
-- TEST_DATA_DIR: /Users/antonioguilhermeferreiraviggiano/Projects/aviggiano/croaring-size-demo/build/_deps/roaring-src/tests/testdata/
-- Building a static library.
-- ROARING_LIB_TYPE: STATIC
-- Library output directory (does not apply to Visual Studio): /Users/antonioguilhermeferreiraviggiano/Projects/aviggiano/croaring-size-demo/build
-- CMAKE_SYSTEM_PROCESSOR: arm64
-- CMAKE_BUILD_TYPE: Release
-- ROARING_DISABLE_X64: OFF
-- ROARING_DISABLE_AVX: ON
-- ROARING_DISABLE_NEON: OFF
-- ROARING_DISABLE_NATIVE: ON
-- ROARING_ARCH: native
-- ROARING_BUILD_STATIC: ON
-- ROARING_LINK_STATIC: OFF
-- ROARING_BUILD_LTO: OFF
-- ROARING_SANITIZE: OFF
-- CMAKE_C_COMPILER: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc
-- CMAKE_C_FLAGS:  -std=c11 -fPIC  -DDISABLEAVX  -Wall -Wextra -Wsign-compare -Wshadow -Wwrite-strings -Wpointer-arith -Winit-self
-- CMAKE_C_FLAGS_DEBUG: -ggdb
-- CMAKE_C_FLAGS_RELEASE: -O3
-- Configuring done
-- Generating done
-- Build files have been written to: /Users/antonioguilhermeferreiraviggiano/Projects/aviggiano/croaring-size-demo/build
Consolidate compiler generated dependencies of target roaring
[ 90%] Built target roaring
Consolidate compiler generated dependencies of target reproc
[ 95%] Building C object CMakeFiles/reproc.dir/main.c.o
[100%] Linking C executable reproc
[100%] Built target reproc
cardinality = 999
size in bytes = 16

[yorkane]

Sorry for the late reply.

Because the memory malloc may cause different binary export results. By using roaring_bitmap_run_optimize method before export binary data to prevent that happen.

By using roaring_bitmap_portable_serialize and roaring_bitmap_portable_deserialize make it cross platform

Now the testing worked as expect.

[yorkane]

I will make a PR later