From 2519f505f2528878e32f99031c460627535fc60f Mon Sep 17 00:00:00 2001 From: Kevin Pedro Date: Thu, 31 Aug 2023 11:55:53 -0500 Subject: [PATCH] avoid unnecessary c-string conversions for pybind interface --- FWCore/Framework/bin/cmsRun.cpp | 13 +++++-------- FWCore/ParameterSetReader/bin/edmConfigHash.cpp | 4 ++-- .../interface/ParameterSetReader.h | 2 +- .../src/ParameterSetReader.cc | 4 ++-- .../interface/MakePyBind11ParameterSets.h | 2 +- .../interface/PyBind11ProcessDesc.h | 2 +- .../src/MakePyBind11ParameterSets.cc | 4 ++-- .../src/PyBind11ProcessDesc.cc | 17 ++++++----------- 8 files changed, 20 insertions(+), 28 deletions(-) diff --git a/FWCore/Framework/bin/cmsRun.cpp b/FWCore/Framework/bin/cmsRun.cpp index d901b20b9c010..335461e304010 100644 --- a/FWCore/Framework/bin/cmsRun.cpp +++ b/FWCore/Framework/bin/cmsRun.cpp @@ -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" @@ -249,13 +248,11 @@ int main(int argc, char* argv[]) { } std::string fileName(vm[kParameterSetOpt].as()); //convert to char*[] - const std::vector& pythonOptValues(vm[kPythonOpt].as>()); - std::vector pythonArgs; + std::vector pythonOptValues(vm[kPythonOpt].as>()); //omit default arg - if (!(pythonOptValues.size() == 1 and pythonOptValues[0] == kPythonOptDefault)) - pythonArgs = - edm::vector_transform(pythonOptValues, [](const auto& arg) { return const_cast(arg.c_str()); }); - pythonArgs.insert(pythonArgs.begin(), const_cast(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); @@ -284,7 +281,7 @@ int main(int argc, char* argv[]) { std::shared_ptr processDesc; try { std::unique_ptr 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; diff --git a/FWCore/ParameterSetReader/bin/edmConfigHash.cpp b/FWCore/ParameterSetReader/bin/edmConfigHash.cpp index e87f0e159ad20..0af76d8206f4f 100644 --- a/FWCore/ParameterSetReader/bin/edmConfigHash.cpp +++ b/FWCore/ParameterSetReader/bin/edmConfigHash.cpp @@ -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 pythonArgs; + std::vector pythonArgs; if (argc == 1) { // Read input from cin into configstring.. @@ -34,7 +34,7 @@ int main(int argc, char** argv) try { } } - std::shared_ptr parameterSet = edm::readConfig(config, pythonArgs.size(), pythonArgs.data()); + std::shared_ptr parameterSet = edm::readConfig(config, pythonArgs); parameterSet->registerIt(); std::cout << parameterSet->id() << std::endl; return 0; diff --git a/FWCore/ParameterSetReader/interface/ParameterSetReader.h b/FWCore/ParameterSetReader/interface/ParameterSetReader.h index 099a3b65c8b92..874cc65949b0c 100644 --- a/FWCore/ParameterSetReader/interface/ParameterSetReader.h +++ b/FWCore/ParameterSetReader/interface/ParameterSetReader.h @@ -7,7 +7,7 @@ namespace edm { std::unique_ptr getPSetFromConfig(const std::string& config); - std::unique_ptr readConfig(std::string const& config, int argc, char* argv[]); + std::unique_ptr readConfig(std::string const& config, const std::vector& args); std::unique_ptr readConfig(std::string const& config); diff --git a/FWCore/ParameterSetReader/src/ParameterSetReader.cc b/FWCore/ParameterSetReader/src/ParameterSetReader.cc index c6ae77decf147..54ef972bd7f36 100644 --- a/FWCore/ParameterSetReader/src/ParameterSetReader.cc +++ b/FWCore/ParameterSetReader/src/ParameterSetReader.cc @@ -8,8 +8,8 @@ std::unique_ptr 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::readConfig(std::string const& config, int argc, char* argv[]) { - return edm::cmspybind11::readConfig(config, argc, argv); +std::unique_ptr edm::readConfig(std::string const& config, const std::vector& args) { + return edm::cmspybind11::readConfig(config, args); } std::unique_ptr edm::readConfig(std::string const& config) { diff --git a/FWCore/PythonParameterSet/interface/MakePyBind11ParameterSets.h b/FWCore/PythonParameterSet/interface/MakePyBind11ParameterSets.h index d672025d120e6..f61545d552c91 100644 --- a/FWCore/PythonParameterSet/interface/MakePyBind11ParameterSets.h +++ b/FWCore/PythonParameterSet/interface/MakePyBind11ParameterSets.h @@ -17,7 +17,7 @@ namespace edm { std::unique_ptr readConfig(std::string const& config); /// same, but with arguments - std::unique_ptr readConfig(std::string const& config, int argc, char* argv[]); + std::unique_ptr readConfig(std::string const& config, const std::vector& args); /// essentially the same as the previous method void makeParameterSets(std::string const& configtext, std::unique_ptr& main); diff --git a/FWCore/PythonParameterSet/interface/PyBind11ProcessDesc.h b/FWCore/PythonParameterSet/interface/PyBind11ProcessDesc.h index 07d07f3c6fcae..0bfcf988456d6 100644 --- a/FWCore/PythonParameterSet/interface/PyBind11ProcessDesc.h +++ b/FWCore/PythonParameterSet/interface/PyBind11ProcessDesc.h @@ -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& args); ~PyBind11ProcessDesc(); diff --git a/FWCore/PythonParameterSet/src/MakePyBind11ParameterSets.cc b/FWCore/PythonParameterSet/src/MakePyBind11ParameterSets.cc index 0b1f30b1afb1e..8310b1b43569d 100644 --- a/FWCore/PythonParameterSet/src/MakePyBind11ParameterSets.cc +++ b/FWCore/PythonParameterSet/src/MakePyBind11ParameterSets.cc @@ -28,8 +28,8 @@ namespace edm { return pythonProcessDesc.parameterSet(); } - std::unique_ptr readConfig(std::string const& config, int argc, char* argv[]) { - PyBind11ProcessDesc pythonProcessDesc(config, true, argc, argv); + std::unique_ptr readConfig(std::string const& config, const std::vector& args) { + PyBind11ProcessDesc pythonProcessDesc(config, true, args); return pythonProcessDesc.parameterSet(); } diff --git a/FWCore/PythonParameterSet/src/PyBind11ProcessDesc.cc b/FWCore/PythonParameterSet/src/PyBind11ProcessDesc.cc index ead2232b2a9ed..d4eb14c89cc97 100644 --- a/FWCore/PythonParameterSet/src/PyBind11ProcessDesc.cc +++ b/FWCore/PythonParameterSet/src/PyBind11ProcessDesc.cc @@ -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& args) : theProcessPSet(), theInterpreter(true) - { edm::python::initializePyBind11Module(); prepareToRead(); { -#if PY_MAJOR_VERSION >= 3 typedef std::unique_ptr WArgUPtr; std::vector v_argv; std::vector 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); }