Skip to content

Commit

Permalink
avoid unnecessary c-string conversions for pybind interface
Browse files Browse the repository at this point in the history
  • Loading branch information
kpedro88 committed Sep 14, 2023
1 parent 9adf546 commit 2519f50
Show file tree
Hide file tree
Showing 8 changed files with 20 additions and 28 deletions.
13 changes: 5 additions & 8 deletions FWCore/Framework/bin/cmsRun.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ PSet script. See notes in EventProcessor.cpp for details about it.
#include "FWCore/Utilities/interface/Presence.h"
#include "FWCore/Utilities/interface/TimingServiceBase.h"
#include "FWCore/Utilities/interface/thread_safety_macros.h"
#include "FWCore/Utilities/interface/transform.h"

#include "TError.h"

Expand Down Expand Up @@ -249,13 +248,11 @@ int main(int argc, char* argv[]) {
}
std::string fileName(vm[kParameterSetOpt].as<std::string>());
//convert to char*[]
const std::vector<std::string>& pythonOptValues(vm[kPythonOpt].as<std::vector<std::string>>());
std::vector<char*> pythonArgs;
std::vector<std::string> pythonOptValues(vm[kPythonOpt].as<std::vector<std::string>>());
//omit default arg
if (!(pythonOptValues.size() == 1 and pythonOptValues[0] == kPythonOptDefault))
pythonArgs =
edm::vector_transform(pythonOptValues, [](const auto& arg) { return const_cast<char*>(arg.c_str()); });
pythonArgs.insert(pythonArgs.begin(), const_cast<char*>(fileName.c_str()));
if (pythonOptValues.size() == 1 and pythonOptValues[0] == kPythonOptDefault)
pythonOptValues.clear();
pythonOptValues.insert(pythonOptValues.begin(), fileName);

if (vm.count(kStrictOpt)) {
//edm::setStrictParsing(true);
Expand Down Expand Up @@ -284,7 +281,7 @@ int main(int argc, char* argv[]) {
std::shared_ptr<edm::ProcessDesc> processDesc;
try {
std::unique_ptr<edm::ParameterSet> parameterSet =
edm::readConfig(fileName, pythonArgs.size(), pythonArgs.data());
edm::readConfig(fileName, pythonOptValues);
processDesc.reset(new edm::ProcessDesc(std::move(parameterSet)));
} catch (edm::Exception const&) {
throw;
Expand Down
4 changes: 2 additions & 2 deletions FWCore/ParameterSetReader/bin/edmConfigHash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
int main(int argc, char** argv) try {
// config can either be a name or a string
std::string config;
std::vector<char*> pythonArgs;
std::vector<std::string> pythonArgs;

if (argc == 1) {
// Read input from cin into configstring..
Expand All @@ -34,7 +34,7 @@ int main(int argc, char** argv) try {
}
}

std::shared_ptr<edm::ParameterSet> parameterSet = edm::readConfig(config, pythonArgs.size(), pythonArgs.data());
std::shared_ptr<edm::ParameterSet> parameterSet = edm::readConfig(config, pythonArgs);
parameterSet->registerIt();
std::cout << parameterSet->id() << std::endl;
return 0;
Expand Down
2 changes: 1 addition & 1 deletion FWCore/ParameterSetReader/interface/ParameterSetReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace edm {

std::unique_ptr<edm::ParameterSet> getPSetFromConfig(const std::string& config);

std::unique_ptr<edm::ParameterSet> readConfig(std::string const& config, int argc, char* argv[]);
std::unique_ptr<edm::ParameterSet> readConfig(std::string const& config, const std::vector<std::string>& args);

std::unique_ptr<edm::ParameterSet> readConfig(std::string const& config);

Expand Down
4 changes: 2 additions & 2 deletions FWCore/ParameterSetReader/src/ParameterSetReader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ std::unique_ptr<edm::ParameterSet> edm::getPSetFromConfig(const std::string& con

//its really the stuff in MakePythonParameterSets that should be in the different namespace
//I'll do that if this setup is ok
std::unique_ptr<edm::ParameterSet> edm::readConfig(std::string const& config, int argc, char* argv[]) {
return edm::cmspybind11::readConfig(config, argc, argv);
std::unique_ptr<edm::ParameterSet> edm::readConfig(std::string const& config, const std::vector<std::string>& args) {
return edm::cmspybind11::readConfig(config, args);
}

std::unique_ptr<edm::ParameterSet> edm::readConfig(std::string const& config) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace edm {
std::unique_ptr<ParameterSet> readConfig(std::string const& config);

/// same, but with arguments
std::unique_ptr<ParameterSet> readConfig(std::string const& config, int argc, char* argv[]);
std::unique_ptr<ParameterSet> readConfig(std::string const& config, const std::vector<std::string>& args);

/// essentially the same as the previous method
void makeParameterSets(std::string const& configtext, std::unique_ptr<ParameterSet>& main);
Expand Down
2 changes: 1 addition & 1 deletion FWCore/PythonParameterSet/interface/PyBind11ProcessDesc.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class PyBind11ProcessDesc {
*/
PyBind11ProcessDesc(std::string const& config, bool isFile);

PyBind11ProcessDesc(std::string const& config, bool isFile, int argc, char* argv[]);
PyBind11ProcessDesc(std::string const& config, bool isFile, const std::vector<std::string>& args);

~PyBind11ProcessDesc();

Expand Down
4 changes: 2 additions & 2 deletions FWCore/PythonParameterSet/src/MakePyBind11ParameterSets.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ namespace edm {
return pythonProcessDesc.parameterSet();
}

std::unique_ptr<ParameterSet> readConfig(std::string const& config, int argc, char* argv[]) {
PyBind11ProcessDesc pythonProcessDesc(config, true, argc, argv);
std::unique_ptr<ParameterSet> readConfig(std::string const& config, const std::vector<std::string>& args) {
PyBind11ProcessDesc pythonProcessDesc(config, true, args);
return pythonProcessDesc.parameterSet();
}

Expand Down
17 changes: 6 additions & 11 deletions FWCore/PythonParameterSet/src/PyBind11ProcessDesc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,31 +31,26 @@ PyBind11ProcessDesc::PyBind11ProcessDesc(std::string const& config, bool isFile)
read(config, isFile);
}

PyBind11ProcessDesc::PyBind11ProcessDesc(std::string const& config, bool isFile, int argc, char* argv[])
PyBind11ProcessDesc::PyBind11ProcessDesc(std::string const& config, bool isFile, const std::vector<std::string>& args)
: theProcessPSet(),
theInterpreter(true)

{
edm::python::initializePyBind11Module();
prepareToRead();
{
#if PY_MAJOR_VERSION >= 3
typedef std::unique_ptr<wchar_t[], decltype(&PyMem_RawFree)> WArgUPtr;
std::vector<WArgUPtr> v_argv;
std::vector<wchar_t*> vp_argv;
v_argv.reserve(argc);
vp_argv.reserve(argc);
for (int i = 0; i < argc; i++) {
v_argv.emplace_back(Py_DecodeLocale(argv[i], nullptr), &PyMem_RawFree);
v_argv.reserve(args.size());
vp_argv.reserve(args.size());
for (size_t i = 0; i < args.size(); i++) {
v_argv.emplace_back(Py_DecodeLocale(args[i].c_str(), nullptr), &PyMem_RawFree);
vp_argv.emplace_back(v_argv.back().get());
}

wchar_t** argvt = vp_argv.data();
#else
char** argvt = argv;
#endif

PySys_SetArgv(argc, argvt);
PySys_SetArgv(args.size(), argvt);
}
read(config, isFile);
}
Expand Down

0 comments on commit 2519f50

Please sign in to comment.