Skip to content

Commit

Permalink
PR improvements
Browse files Browse the repository at this point in the history
* string ref counter wrapped in class and made static
* removed manual addRef()
* assuming ref=1 for untracked refs
* synchronized read and write access to ref counter
  • Loading branch information
corepointer committed Jul 29, 2024
1 parent 05d217d commit f0a0881
Show file tree
Hide file tree
Showing 14 changed files with 159 additions and 60 deletions.
11 changes: 9 additions & 2 deletions UserConfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"explain_select_matrix_repr": false,
"explain_type_adaptation": false,
"explain_vectorized": false,
"explain_obj_ref_mgnt": false,
"explain_obj_ref_mgnt": true,
"explain_mlir_codegen": false,
"taskPartitioningScheme": "STATIC",
"numberOfThreads": -1,
Expand All @@ -31,7 +31,7 @@
"daphnedsl_import_paths": {},
"force_cuda": false,
"logging": [
{ "log-level-limit": "ERROR" },
{ "log-level-limit": "DEBUG" },
{
"comment": "This configuration controls logging in the GPU compiler pass only",
"name": "compiler::cuda",
Expand Down Expand Up @@ -66,6 +66,13 @@
"level": "INFO",
"filename": "",
"format": "%^[%n %L]:%$ %v"
},
{
"comment": "DAPHNE runtime logs",
"name": "runtime",
"level": "DEBUG",
"filename": "",
"format": "%^[%n %L]:%$ %v"
}
],
"sparsity_threshold": 0.25
Expand Down
6 changes: 5 additions & 1 deletion src/compiler/lowering/InsertDaphneContextPass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <ir/daphneir/Passes.h>
#include <util/KernelDispatchMapping.h>
#include <util/Statistics.h>
#include <util/StringRefCount.h>

#include <mlir/Pass/Pass.h>

Expand Down Expand Up @@ -59,7 +60,10 @@ void InsertDaphneContextPass::runOnOperation()
reinterpret_cast<uint64_t>(&KernelDispatchMapping::instance())),
builder.create<daphne::ConstantOp>(
loc,
reinterpret_cast<uint64_t>(&Statistics::instance())));
reinterpret_cast<uint64_t>(&Statistics::instance())),
builder.create<daphne::ConstantOp>(
loc,
reinterpret_cast<uint64_t>(&StringRefCounter::instance())));
#ifdef USE_CUDA
if(user_config.use_cuda) {
builder.create<daphne::CreateCUDAContextOp>(loc);
Expand Down
3 changes: 2 additions & 1 deletion src/ir/daphneir/DaphneOps.td
Original file line number Diff line number Diff line change
Expand Up @@ -1474,7 +1474,8 @@ def Daphne_CreateDaphneContextOp : Daphne_Op<"createDaphneContext"> {
let arguments = (ins
UI64 : $userConfigPtr,
UI64 : $dispatchMappingPtr,
UI64 : $statisticsPtr);
UI64 : $statisticsPtr,
UI64 : $stringRefCountPtr);
let results = (outs DaphneContext : $ctx);
}

Expand Down
35 changes: 6 additions & 29 deletions src/runtime/local/context/DaphneContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <api/cli/DaphneUserConfig.h>
#include <util/KernelDispatchMapping.h>
#include <util/Statistics.h>
#include <util/StringRefCount.h>

#include <vector>
#include <iostream>
Expand Down Expand Up @@ -58,7 +59,6 @@ struct DaphneContext {
std::vector<std::unique_ptr<IContext>> cuda_contexts;
std::vector<std::unique_ptr<IContext>> fpga_contexts;


std::unique_ptr<IContext> distributed_context;

/**
Expand All @@ -71,17 +71,15 @@ struct DaphneContext {
DaphneUserConfig& config;
KernelDispatchMapping& dispatchMapping;
Statistics& stats;
StringRefCounter& stringRefCount;

std::shared_ptr<spdlog::logger> logger;

// This map keeps track of string allocations to be able to remove them when they are not needed anymore
std::map<uintptr_t, size_t> stringRefCount;
std::mutex mtxStrRefCnt;

explicit DaphneContext(DaphneUserConfig &config,
KernelDispatchMapping &dispatchMapping,
Statistics &stats)
: config(config), dispatchMapping(dispatchMapping), stats(stats) {
Statistics &stats,
StringRefCounter& stringRefCnt)
: config(config), dispatchMapping(dispatchMapping), stats(stats), stringRefCount(stringRefCnt) {
logger = spdlog::get("runtime");
}

Expand All @@ -94,15 +92,6 @@ struct DaphneContext {
}
cuda_contexts.clear();
fpga_contexts.clear();

if(!stringRefCount.empty()) {
// This should not happen
std::cerr << "Deleting " << stringRefCount.size() << " remaining string refs in ~DaphneContext()" << std::endl;
logger->debug("Deleting {} remaining string refs in ~DaphneContext()", stringRefCount.size());
// for (auto &ref: stringRefCount) {
// delete[] reinterpret_cast<char *>(ref.first);
// }
}
}

#ifdef USE_CUDA
Expand Down Expand Up @@ -136,17 +125,5 @@ struct DaphneContext {

[[nodiscard]] DaphneUserConfig &getUserConfig() const { return config; }

/**
* @brief This method adds a numeric representation of a char* and an initial reference count
* to delete the string/char* later on in the DecRef kernel.
*
* @param str The char* to keep track of
*
*/
void addStrRef(const char* str) {
mtxStrRefCnt.lock();
auto ptr = reinterpret_cast<uintptr_t>(str);
stringRefCount.insert({ptr, 1});
mtxStrRefCnt.unlock();
}

};
2 changes: 0 additions & 2 deletions src/runtime/local/kernels/CastSca.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,6 @@ struct CastSca<const char *, VTArg> {
std::string str = std::to_string(arg).c_str();
const size_t len = str.length();
char * res = new char[len + 1]();
// reference counting for removal later
ctx->addStrRef(res);
strncpy(res, str.c_str(), len);
res[len] = 0;
return res;
Expand Down
2 changes: 0 additions & 2 deletions src/runtime/local/kernels/Concat.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ void concat(char *& res, const char * lhs, const char * rhs, DCTX(ctx)) {

if(res == nullptr) {
res = new char[lenRes + 1];
// reference counting for removal later
ctx->addStrRef(res);
}

std::memcpy(res , lhs, lenLhs);
Expand Down
5 changes: 3 additions & 2 deletions src/runtime/local/kernels/CreateDaphneContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@
#include "util/KernelDispatchMapping.h"

void createDaphneContext(DaphneContext *&res, uint64_t configPtr,
uint64_t dispatchMappingPtr, uint64_t statisticsPtr) {
uint64_t dispatchMappingPtr, uint64_t statisticsPtr, uint64_t stringRefCountPtr) {
auto config = reinterpret_cast<DaphneUserConfig *>(configPtr);
auto dispatchMapping =
reinterpret_cast<KernelDispatchMapping *>(dispatchMappingPtr);
auto statistics = reinterpret_cast<Statistics *>(statisticsPtr);
auto stringRefCounter = reinterpret_cast<StringRefCounter*>(stringRefCountPtr);
if (config->log_ptr != nullptr)
config->log_ptr->registerLoggers();
res = new DaphneContext(*config, *dispatchMapping, *statistics);
res = new DaphneContext(*config, *dispatchMapping, *statistics, *stringRefCounter);
}
2 changes: 1 addition & 1 deletion src/runtime/local/kernels/CreateDaphneContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,4 @@
// Convenience function
// ****************************************************************************
void createDaphneContext(DaphneContext *&res, uint64_t configPtr,
uint64_t dispatchMappingPtr, uint64_t statisticsPtr);
uint64_t dispatchMappingPtr, uint64_t statisticsPtr, uint64_t stringRefCountPtr);
16 changes: 4 additions & 12 deletions src/runtime/local/kernels/DecRef.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,10 @@ struct DecRef<Structure> {
template<>
struct DecRef<char> {
static void apply(const char* arg, DCTX(ctx)) {
auto ptr = reinterpret_cast<uintptr_t>(arg);
if(auto found = ctx->stringRefCount.find(ptr); found != ctx->stringRefCount.end()) {
std::cerr << "DecRef: " << ptr << " (found)" << std::endl;
found->second--;
if(found->second == 0) {
ctx->logger->debug("DecRef deleting: {}", arg);
// delete [] reinterpret_cast<char *>(found->first);
ctx->stringRefCount.erase(found);
}
}
else {
std::cerr << "DecRef: " << ptr << " (not found)" << std::endl;

if(!ctx->stringRefCount.dec(arg)) {
std::cerr << "DecRef: " << reinterpret_cast<uintptr_t>(arg) << " (not found) - assuming count=1 and deleting \"" << arg << "\"" << std::endl;
delete [] arg;
}
}
};
Expand Down
13 changes: 5 additions & 8 deletions src/runtime/local/kernels/IncRef.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,11 @@ struct IncRef<Structure> {
template<>
struct IncRef<char> {
static void apply(const char* arg, DCTX(ctx)) {
auto ptr = reinterpret_cast<uintptr_t>(arg);
if(auto found = ctx->stringRefCount.find(ptr); found != ctx->stringRefCount.end()) {
std::cerr << "IncRef: " << ptr << " (found)" << std::endl;
ctx->logger->info("IncRef: {}", arg);
found->second++;
}
else {
std::cerr << "IncRef: " << ptr << " (not found)" << std::endl;

if(!ctx->stringRefCount.inc(arg)) {
std::cerr << "IncRef: " << reinterpret_cast<uintptr_t>(arg) << " (not found) - adding \"" << arg << "\" with count=2" << std::endl;
// reference counting for removal later
ctx->stringRefCount.add(arg);
}
}
};
Expand Down
4 changes: 4 additions & 0 deletions src/runtime/local/kernels/kernels.json
Original file line number Diff line number Diff line change
Expand Up @@ -933,6 +933,10 @@
{
"type": "uint64_t",
"name": "statisticsPtr"
},
{
"type": "uint64_t",
"name": "stringRefCountPtr"
}
]
},
Expand Down
2 changes: 2 additions & 0 deletions src/util/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ add_library(Util
preprocessor_defs.h
Statistics.h
Statistics.cpp
StringRefCount.h
StringRefCount.cpp
)

target_link_libraries(Util PRIVATE spdlog::spdlog)
Expand Down
57 changes: 57 additions & 0 deletions src/util/StringRefCount.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* Copyright 2024 The DAPHNE Consortium
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#include "StringRefCount.h"

void StringRefCounter::add(const char* arg) {
auto ptr = reinterpret_cast<uintptr_t>(arg);
const std::lock_guard<std::mutex> lock(mtxStrRefCnt);
stringRefCount.insert({ptr, 2});
logger->info("StringRefCount: Added ptr={}; arg={}", ptr, arg);
}

bool StringRefCounter::inc(const char* arg) {
auto ptr = reinterpret_cast<uintptr_t>(arg);
const std::lock_guard<std::mutex> lock(mtxStrRefCnt);
if(auto found = stringRefCount.find(ptr); found != stringRefCount.end()) {
found->second++;
std::cerr << "IncRef: " << ptr << " (found) - count incremented to " << found->second << std::endl;
logger->info("IncRef: ptr={}; arg={}", ptr, arg);
return true;
}
return false;
}

bool StringRefCounter::dec(const char* arg) {
auto ptr = reinterpret_cast<uintptr_t>(arg);
const std::lock_guard<std::mutex> lock(mtxStrRefCnt);
if(auto found = stringRefCount.find(ptr); found != stringRefCount.end()) {
found->second--;
std::cerr << "DecRef: " << ptr << " (found) - count decremented to " << found->second << std::endl;
if(found->second == 1) {
logger->debug("Removing from StringRefCounter: ptr={}; arg={}", ptr, arg);
// delete [] reinterpret_cast<char *>(found->first);
stringRefCount.erase(found);
}
return true;
}
return false;
}

StringRefCounter& StringRefCounter::instance() {
static StringRefCounter INSTANCE;
return INSTANCE;
}
61 changes: 61 additions & 0 deletions src/util/StringRefCount.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* Copyright 2024 The DAPHNE Consortium
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#pragma once

#include <spdlog/spdlog.h>

#include <iostream>
#include <map>
#include <memory>

class StringRefCounter {

std::shared_ptr<spdlog::logger> logger;

// This map keeps track of string allocations to be able to remove them when they are not needed anymore
std::map<uintptr_t, size_t> stringRefCount;
std::mutex mtxStrRefCnt;

public:
StringRefCounter() {
logger = spdlog::get("runtime");
}

~StringRefCounter() {
if(!stringRefCount.empty()) {
// This should not happen
std::cerr << "Deleting " << stringRefCount.size() << " remaining string refs in ~DaphneContext()" << std::endl;
logger->warn("{} string refs still present while destroying DaphneContext - this should not happen.", stringRefCount.size());
}
}

/**
* @brief This method adds a numeric representation of a char* and an initial reference count
* to delete the string/char* later on in the DecRef kernel.
*
* @param str The char* to keep track of
*
*/
void add(const char* arg);

bool inc(const char* arg);

bool dec(const char* arg);

static StringRefCounter& instance();
};

0 comments on commit f0a0881

Please sign in to comment.