Skip to content

add nccl env intercept example code #269

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MC952-arch merged 2 commits into from
Oct 16, 2025
Merged

add nccl env intercept example code #269

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f765100
add nccl env intercept code
yzhang35 Oct 14, 2025
538f93f
add nccl env intercept code
yzhang35 Oct 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions flagcx/adaptor/tuner/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Compiler and flags for C++
CXX = g++
CXXFLAGS = -fPIC -Wall -Wextra -O2 -std=c++11
LDFLAGS = -shared -pthread

# Target shared library name
TARGET = libnccl_param.so

# Source files
SOURCES = nccl_param.cc

# Object files (automatically generated from source files)
OBJECTS = $(SOURCES:.cc=.o)

# Default target
all: $(TARGET)

# Build the shared library
$(TARGET): $(OBJECTS)
$(CXX) $(LDFLAGS) -o $@ $(OBJECTS)

# Compile C++ source files to object files
%.o: %.cc
$(CXX) $(CXXFLAGS) -c $< -o $@

# Clean up build files
clean:
rm -f $(OBJECTS) $(TARGET)

# Phony targets
.PHONY: all clean
33 changes: 33 additions & 0 deletions flagcx/adaptor/tuner/nccl_param.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// This is a example of intercept NCCL environment getter functions.
#include <stdio.h>

@gemini-code-assist gemini-code-assist Bot Oct 14, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The <stdio.h> header is not used in this file and can be removed to keep the includes clean. Additionally, for better C++ code style, consider using the C++-style headers <cstdlib> and <cstring> instead of <stdlib.h> and <cstring>.

#include <cstring>
#include <string>
#include <cstdint>
#include <pthread.h>
#include <stdlib.h> // for setenv, getenv

static void ncclLoadParam(char const* env, int64_t deftVal, int64_t* value) {
static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
pthread_mutex_lock(&mutex);
const char* str = getenv(env);
*value = deftVal;
if (str && strlen(str) > 0) {
try {
*value = std::stoll(str);
} catch (const std::exception& e) {
*value = deftVal;
}
}
pthread_mutex_unlock(&mutex);
}
Comment on lines +9 to +22

@gemini-code-assist gemini-code-assist Bot Oct 14, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

This function can be improved for better C++11 style and clarity:

  1. Consider using std::mutex and std::lock_guard instead of pthreads for thread synchronization. This is more idiomatic C++ and safer as it uses RAII to ensure the mutex is always unlocked. This would require including <mutex> and removing <pthread.h>.
  2. The assignment *value = deftVal; inside the catch block is redundant because *value has already been set to deftVal on line 13.

Here is a suggested refactoring that applies both points:

static void ncclLoadParam(char const* env, int64_t deftVal, int64_t* value) {
  static std::mutex param_mutex;
  std::lock_guard<std::mutex> lock(param_mutex);
  const char* str = getenv(env);
  *value = deftVal;
  if (str && strlen(str) > 0) {
    try {
      *value = std::stoll(str);
    } catch (const std::exception& e) {
      // If parsing fails, *value keeps deftVal assigned earlier.
    }
  }
}


#define NCCL_PARAM(name, env, deftVal) \
int64_t ncclParam##name() { \
int64_t value = INT64_MIN; \
ncclLoadParam("NCCL_" env, deftVal, &value); \
return value; \
}

// In order to intercept a NCCL environment getter function, add an additional line of NCCL_PRARM here.

@gemini-code-assist gemini-code-assist Bot Oct 14, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

There's a typo in this explanatory comment. NCCL_PRARM should be NCCL_PARAM to match the macro defined above. This will help users who read this example.

// In order to intercept a NCCL environment getter function, add an additional line of NCCL_PARAM here.

// Below is an example of intercepting NCCL_P2P_NVL_CHUNKSIZE env.
NCCL_PARAM(P2pNvlChunkSize, "P2P_NVL_CHUNKSIZE", (1 << 19)); /* 512 kB */
4 changes: 2 additions & 2 deletions flagcx/core/flagcx_tuner.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,11 @@ struct flagcxTunerContext {
static struct flagcxEnvConfig config1 = {
"defaultConfig1",
1,
{FLAGCX_ENV_TYPE_COLL, "NCCL_P2P_NVL_CHUNKSIZE", "1024", "524288"}};
{FLAGCX_ENV_TYPE_CREATION, "NCCL_P2P_NVL_CHUNKSIZE", "1024", "524288"}};
static struct flagcxEnvConfig config2 = {
"defaultConfig2",
1,
{FLAGCX_ENV_TYPE_COLL, "NCCL_P2P_NVL_CHUNKSIZE", "524288", "524288"}};
{FLAGCX_ENV_TYPE_CREATION, "NCCL_P2P_NVL_CHUNKSIZE", "524288", "524288"}};

bool operator<(const struct flagcxCommTag &lhs,
const struct flagcxCommTag &rhs) {
Expand Down