Skip to content

Conversation

git-hulk
Copy link
Member

@git-hulk git-hulk commented May 30, 2025

This closes #3000.

This only enables profiling at the compile stage, but since we set prof_active to false, the profiling is not active, so it won't incur any overhead for now. We can enable the profiling by setting the environment MALLOC_CONF like:

MALLOC_CONF=prof:true,prof_active:true,lg_prof_interval:30,lg_prof_sample:21,prof_prefix:/tmp/jeprof ./kvrocks

And it will produce the jemalloc under the /tmp directory like the following:

jeprof.2359.0.i0.heap
jeprof.2359.1.i1.heap  
jeprof.2359.3.i3.heap

This only enables the profiling at the compile stage, but the profiling
is not active since we set `prof_active` to false, so it won't bring any
overhead for now. We can enable the profiling by setting the environment
MALLOC_CONF.
@git-hulk git-hulk requested a review from Copilot May 30, 2025 13:31
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enables the jemalloc profiling option at compile time while keeping profiling inactive by default, allowing future activation without performance overhead.

  • Introduces a CMake flag that conditionally adds the profiling option.
  • Adjusts the configure command to include the profiling flag when profiling is enabled.

set(ENABLE_JEMALLOC_PROFILING "--enable-prof")
# jemalloc profiling is enabled by default, but we can disable it and
# enable it at runtime by setting the environment variable.
set(JEMALLOC_CONFIG_MALLOC_CONF "prof:true,prof_active:false")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a CMake variable and will not be passed to the build phase of jemalloc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh? I see Jemalloc's configure.ac recognizes this environment, so I would like to test if CMake could pass this.
https://github.com/jemalloc/jemalloc/blob/edaab8b3ad752a845019985062689551cd6315c1/configure.ac#L1278

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems it can also be configured in runtime via MALLOC_CONF env var, so we can ignore it?

Copy link
Member Author

@git-hulk git-hulk May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line mainly uses to set prof_active to false by default, so that it won’t collect the profiling unless user sets the MALLOC_CONF to enable it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested on my side, the heap profiling won't run if we didn't enable opt.prof=true via MALLOC_CONF, so we can remove this line.

@git-hulk git-hulk requested a review from PragmaTwice May 30, 2025 14:39
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
40.6% Coverage on New Code (required ≥ 50%)

See analysis details on SonarQube Cloud

@PragmaTwice
Copy link
Member

PragmaTwice commented May 30, 2025

Is there any performance impact of this flag? e.g. through redis-benchmark.

@git-hulk
Copy link
Member Author

git-hulk commented May 31, 2025

Is there any performance impact of this flag? e.g. through redis-benchmark.

In summary, there's no impact on performance. Many other projects are also enabled when compiling, like ClickHouse and MySQL(Percona),

The first one is running without enable the --enable-prof when compiling.

hulk@instance-20241021-041032:~$ redis-benchmark -c -p 6666 -n 1000000 -r 1000000 -t set -d 128
====== 6666 -n 1000000 -r 1000000 -t set -d 128 ======
  100000 requests completed in 4.52 seconds
  0 parallel clients
  3 bytes payload
  keep alive: 1
  host configuration "save": 900 1 300 10 60 10000
  host configuration "appendonly": no
  multi-thread: no

99.62% <= 0.1 milliseconds
99.95% <= 0.2 milliseconds
100.00% <= 0.3 milliseconds
100.00% <= 0.7 milliseconds
100.00% <= 1.0 milliseconds
22114.11 requests per second

The second one is running with --enable-prof, but prof_active is false.

hulk@instance-20241021-041032:~$ redis-benchmark -c -p 6666 -n 1000000 -r 1000000 -t set -d 128
====== 6666 -n 1000000 -r 1000000 -t set -d 128 ======
  100000 requests completed in 4.52 seconds
  0 parallel clients
  3 bytes payload
  keep alive: 1
  host configuration "save": 900 1 300 10 60 10000
  host configuration "appendonly": no
  multi-thread: no

99.62% <= 0.1 milliseconds
99.95% <= 0.2 milliseconds
99.99% <= 0.3 milliseconds
100.00% <= 0.4 milliseconds
100.00% <= 0.5 milliseconds
100.00% <= 0.6 milliseconds
100.00% <= 1.0 milliseconds
22128.79 requests per second

The last one is running with --enable-prof and prof_active is true.

hulk@instance-20241021-041032:~$ redis-benchmark -c -p 6666 -n 1000000 -r 1000000 -t set -d 128
====== 6666 -n 1000000 -r 1000000 -t set -d 128 ======
  100000 requests completed in 4.54 seconds
  0 parallel clients
  3 bytes payload
  keep alive: 1
  host configuration "save": 900 1 300 10 60 10000
  host configuration "appendonly": no
  multi-thread: no

99.51% <= 0.1 milliseconds
99.94% <= 0.2 milliseconds
99.99% <= 0.3 milliseconds
100.00% <= 0.4 milliseconds
100.00% <= 0.4 milliseconds
22011.88 requests per second

@git-hulk git-hulk merged commit db08209 into apache:unstable May 31, 2025
35 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enabling the option –enable-prof at compile time
2 participants