Skip to content

perf: intern Dict keys as uint16_t, make common_properties DictKey constants #11

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

Open
bddap wants to merge 10 commits into
base: master
Choose a base branch
from
Open

perf: intern Dict keys as uint16_t, make common_properties DictKey constants #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f62f675
Dict: use interned uint16_t keys instead of std::string keys
Feb 18, 2026
722d901
Dict: make common_properties hold DictKey constants directly
Feb 26, 2026
317d2fc
Fix internKey cache false hit on empty strings
Feb 26, 2026
9e5ddc5
bench: opt-dict-int-keys vs master (3×100)
Feb 28, 2026
86219a7
bench: opt-dict-int-keys vs master (5×100)
Mar 26, 2026
993c783
bench: opt-dict-int-keys vs master (5×100)
Mar 27, 2026
beb0d54
Add 3-arg streamReadProp overload for Dict::Pair
Mar 27, 2026
1ca8593
Fix MaeWriter Dict iteration: use getData() for string-key pairs
Mar 27, 2026
ae12870
bench: opt-dict-int-keys vs master (5×100)
Mar 27, 2026
9cf814c
bench: opt-dict-int-keys vs master (16×800)
Mar 29, 2026
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
8 changes: 4 additions & 4 deletions Code/GraphMol/ChemReactions/PreprocessRxn.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,24 +42,24 @@ namespace RDKit {

RDKIT_CHEMREACTIONS_EXPORT bool preprocessReaction(
ChemicalReaction &rxn,
const std::string_view &propName = common_properties::molFileValue);
const std::string_view &propName = kWellKnownKeys[common_properties::molFileValue]);

RDKIT_CHEMREACTIONS_EXPORT bool preprocessReaction(
ChemicalReaction &rxn, unsigned int &numWarnings, unsigned int &numErrors,
std::vector<std::vector<std::pair<unsigned int, std::string>>>
&reactantLabels,
const std::string_view &propName = common_properties::molFileValue);
const std::string_view &propName = kWellKnownKeys[common_properties::molFileValue]);

RDKIT_CHEMREACTIONS_EXPORT bool preprocessReaction(
ChemicalReaction &rxn, const std::map<std::string, ROMOL_SPTR> &queries,
const std::string_view &propName = common_properties::molFileValue);
const std::string_view &propName = kWellKnownKeys[common_properties::molFileValue]);

RDKIT_CHEMREACTIONS_EXPORT bool preprocessReaction(
ChemicalReaction &rxn, unsigned int &numWarnings, unsigned int &numErrors,
std::vector<std::vector<std::pair<unsigned int, std::string>>>
&reactantLabels,
const std::map<std::string, ROMOL_SPTR> &queries,
const std::string_view &propName = common_properties::molFileValue);
const std::string_view &propName = kWellKnownKeys[common_properties::molFileValue]);
} // namespace RDKit

#endif
6 changes: 3 additions & 3 deletions Code/GraphMol/ChemReactions/ReactionUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ bool hasReactionAtomMapping(const ChemicalReaction &rxn) {
for (; begin != end; ++begin) {
const ROMol &reactant = *begin->get();
if (MolOps::getNumAtomsWithDistinctProperty(
reactant, common_properties::molAtomMapNumber)) {
reactant, keyToString(common_properties::molAtomMapNumber))) {
return true;
}
}
Expand All @@ -150,7 +150,7 @@ bool hasReactionAtomMapping(const ChemicalReaction &rxn) {
for (; begin != end; ++begin) {
const ROMol &reactant = *begin->get();
if (MolOps::getNumAtomsWithDistinctProperty(
reactant, common_properties::molAtomMapNumber)) {
reactant, keyToString(common_properties::molAtomMapNumber))) {
return true;
}
}
Expand All @@ -159,7 +159,7 @@ bool hasReactionAtomMapping(const ChemicalReaction &rxn) {

bool isReactionTemplateMoleculeAgent(const ROMol &mol, double agentThreshold) {
unsigned numMappedAtoms = MolOps::getNumAtomsWithDistinctProperty(
mol, common_properties::molAtomMapNumber);
mol, keyToString(common_properties::molAtomMapNumber));
unsigned numAtoms = mol.getNumHeavyAtoms();
return !(numAtoms > 0u && static_cast<double>(numMappedAtoms) /
static_cast<double>(numAtoms) >=
Expand Down
2 changes: 1 addition & 1 deletion Code/GraphMol/ChemReactions/SanitizeRxn.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ namespace RxnOps {
// molFileRLabel ==> unsigned int
namespace {
template <class T>
T getMaxProp(ChemicalReaction &rxn, const std::string_view &prop) {
T getMaxProp(ChemicalReaction &rxn, DictKey prop) {
T max_atom = (T)0;
for (auto it = rxn.beginReactantTemplates(); it != rxn.endReactantTemplates();
++it) {
Expand Down
2 changes: 1 addition & 1 deletion Code/GraphMol/ChemReactions/Wrap/rdChemReactions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1246,7 +1246,7 @@ One unrecognized group type in a comma-separated list makes the whole thing fail
python::def(
"PreprocessReaction", RDKit::PreprocessReaction,
(python::arg("reaction"), python::arg("queries") = python::dict(),
python::arg("propName") = RDKit::common_properties::molFileValue),
python::arg("propName") = RDKit::kWellKnownKeys[RDKit::common_properties::molFileValue]),
Copy link
Copy Markdown

@bp-kelley bp-kelley Apr 13, 2026

Choose a reason for hiding this comment

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

I think this would be better as a generic function to hide the implementations.

std::string GetPropName(int tag)
int GetPropTag(const std::string &propname)

docString.c_str());

python::enum_<RDKit::RxnOps::SanitizeRxnFlags>("SanitizeFlags")
Expand Down
6 changes: 3 additions & 3 deletions Code/GraphMol/Descriptors/MolSurf.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,9 @@ double getTPSAAtomContribs(const ROMol &mol, std::vector<double> &Vi,
TEST_ASSERT(Vi.size() >= mol.getNumAtoms());
double res = 0;
std::string pname =
(boost::format("%s-%s") % common_properties::_tpsa % includeSandP).str();
(boost::format("%s-%s") % keyToString(common_properties::_tpsa) % includeSandP).str();
std::string contribsName =
(boost::format("%s-%s") % common_properties::_tpsaAtomContribs %
(boost::format("%s-%s") % keyToString(common_properties::_tpsaAtomContribs) %
includeSandP)
.str();
if (!force && mol.hasProp(contribsName)) {
Expand Down Expand Up @@ -346,7 +346,7 @@ double getTPSAAtomContribs(const ROMol &mol, std::vector<double> &Vi,
}
double calcTPSA(const ROMol &mol, bool force, bool includeSandP) {
std::string pname =
(boost::format("%s-%s") % common_properties::_tpsa % includeSandP).str();
(boost::format("%s-%s") % keyToString(common_properties::_tpsa) % includeSandP).str();
if (!force && mol.hasProp(pname)) {
double res;
mol.getProp(pname, res);
Expand Down
6 changes: 3 additions & 3 deletions Code/GraphMol/FileParsers/MaeWriter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,9 @@ void copyProperties(
// since we don't want to export these.
origin.clearComputedProps();

for (const auto &prop : origin.getDict()) {
for (const auto &prop : origin.getDict().getData()) {
// Skip the property holding the names of the computed properties
if (prop.key == detail::computedPropName) {
if (prop.key == keyToString(detail::computedPropName)) {
continue;
}

Expand All @@ -164,7 +164,7 @@ void copyProperties(
case RDTypeTag::IntTag:
case RDTypeTag::UnsignedIntTag: {
auto propName = prop.key;
if (prop.key == common_properties::_MolFileRLabel) {
if (prop.key == keyToString(common_properties::_MolFileRLabel)) {
propName = MAE_RGROUP_LABEL;
} else if (!std::regex_match(prop.key, MMCT_PROP_REGEX)) {
propName.insert(0, "i_rdkit_");
Expand Down
2 changes: 1 addition & 1 deletion Code/GraphMol/FileParsers/MolFileWriter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1315,7 +1315,7 @@ std::string outputMolToMolBlock(const RWMol &tmol, int confId,
if (conf->is3D()) {
ss << "3D";
} else {
ss << common_properties::TWOD;
ss << keyToString(common_properties::TWOD);
}
}
res += ss.str();
Expand Down
6 changes: 3 additions & 3 deletions Code/GraphMol/FileParsers/SDWriter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,10 @@ void _MolToSDStream(std::ostream *dp_ostream, const ROMol &mol, int confId,
STR_VECT_CI pi;
for (pi = properties.begin(); pi != properties.end(); pi++) {
// ignore any of the following properties
if (((*pi) == RDKit::detail::computedPropName) ||
((*pi) == common_properties::_Name) || ((*pi) == "_MolFileInfo") ||
if (((*pi) == keyToString(RDKit::detail::computedPropName)) ||
((*pi) == keyToString(common_properties::_Name)) || ((*pi) == "_MolFileInfo") ||
((*pi) == "_MolFileComments") ||
((*pi) == common_properties::_MolFileChiralFlag)) {
((*pi) == keyToString(common_properties::_MolFileChiralFlag))) {
continue;
}

Expand Down
2 changes: 1 addition & 1 deletion Code/GraphMol/FileParsers/TDTMolSupplier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ std::unique_ptr<RWMol> TDTMolSupplier::parseMol(std::string inLine) {
boost::trim_if(propName, boost::is_any_of(" \t"));
startP = endP + 1;

if (propName == common_properties::TWOD && d_params.confId2D >= 0) {
if (propName == keyToString(common_properties::TWOD) && d_params.confId2D >= 0) {
std::string rest = inLine.substr(startP, inLine.size() - startP);
std::vector<double> coords;
TDTParseUtils::ParseNumberList(rest, coords, dp_inStream);
Expand Down
6 changes: 3 additions & 3 deletions Code/GraphMol/FileParsers/TDTWriter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,10 @@ void TDTWriter::write(const ROMol &mol, int confId) {
STR_VECT_CI pi;
for (pi = properties.begin(); pi != properties.end(); pi++) {
// ignore any of the following properties
if (((*pi) == RDKit::detail::computedPropName) ||
((*pi) == common_properties::_Name) || ((*pi) == "_MolFileInfo") ||
if (((*pi) == keyToString(RDKit::detail::computedPropName)) ||
((*pi) == keyToString(common_properties::_Name)) || ((*pi) == "_MolFileInfo") ||
((*pi) == "_MolFileComments") ||
((*pi) == common_properties::_MolFileChiralFlag)) {
((*pi) == keyToString(common_properties::_MolFileChiralFlag))) {
continue;
}

Expand Down
2 changes: 1 addition & 1 deletion Code/GraphMol/FileParsers/TplFileParser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ Conformer *ParseConfData(std::istream &inStream, unsigned int &line, RWMol *mol,
throw FileParseException(errout.str());
}
std::ostringstream propName;
propName << "Conf_" << mol->getNumConformers() << common_properties::_Name;
propName << "Conf_" << mol->getNumConformers() << keyToString(common_properties::_Name);
mol->setProp(propName.str(),
boost::trim_copy(tempStr.substr(4, tempStr.size() - 4)));

Expand Down
4 changes: 2 additions & 2 deletions Code/GraphMol/FileParsers/file_parsers_catch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6204,8 +6204,8 @@ void check_roundtripped_properties(RDProps &original, RDProps &roundtrip) {
REQUIRE(std::includes(roundtripPropNames.begin(), roundtripPropNames.end(),
originalPropNames.begin(), originalPropNames.end()));

for (const auto &o : original.getDict()) {
if (o.key == detail::computedPropName) {
for (const auto &o : original.getDict().getData()) {
if (o.key == keyToString(detail::computedPropName)) {
continue;
}

Expand Down
21 changes: 20 additions & 1 deletion Code/GraphMol/FragCatalog/FragCatalogEntry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,37 +66,56 @@ class RDKIT_FRAGCATALOG_EXPORT FragCatalogEntry
// REVIEW: this should be removed?
std::string getSmarts() { return ""; }

// FUnctions on the property dictionary
template <typename T>
void setProp(const std::string_view &key, T &val) const {
dp_props->setVal(key, val);
}
template <typename T>
void setProp(DictKey key, T &val) const {
dp_props->setVal(key, val);
}

void setProp(const std::string_view &key, int val) const {
dp_props->setVal(key, val);
}
void setProp(DictKey key, int val) const { dp_props->setVal(key, val); }

void setProp(const std::string_view &key, float val) const {
dp_props->setVal(key, val);
}
void setProp(DictKey key, float val) const { dp_props->setVal(key, val); }

void setProp(const std::string_view &key, std::string &val) const {
dp_props->setVal(key, val);
}
void setProp(DictKey key, std::string &val) const {
dp_props->setVal(key, val);
}

template <typename T>
void getProp(const std::string_view &key, T &res) const {
dp_props->getVal(key, res);
}
template <typename T>
void getProp(DictKey key, T &res) const {
dp_props->getVal(key, res);
}

bool hasProp(const std::string_view &key) const {
if (!dp_props) {
return false;
}
return dp_props->hasVal(key);
}
bool hasProp(DictKey key) const {
if (!dp_props) {
return false;
}
return dp_props->hasVal(key);
}

void clearProp(const std::string_view &key) const { dp_props->clearVal(key); }
void clearProp(DictKey key) const { dp_props->clearVal(key); }

void toStream(std::ostream &ss) const override;
std::string Serialize() const override;
Expand Down
22 changes: 11 additions & 11 deletions Code/GraphMol/MolPickler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,26 +168,26 @@ class PropTracker {
// the properties themselves are stored as std::int8_t
static constexpr std::array<std::pair<std::string_view, std::uint16_t>, 5>
explicitBondProps{{
{RDKit::common_properties::_MolFileBondType, 0x1},
{RDKit::common_properties::_MolFileBondStereo, 0x2},
{RDKit::common_properties::_MolFileBondCfg, 0x4},
{RDKit::common_properties::_MolFileBondQuery, 0x8},
{RDKit::common_properties::molStereoCare, 0x10},
{kWellKnownKeys[common_properties::_MolFileBondType], 0x1},
{kWellKnownKeys[common_properties::_MolFileBondStereo], 0x2},
{kWellKnownKeys[common_properties::_MolFileBondCfg], 0x4},
{kWellKnownKeys[common_properties::_MolFileBondQuery], 0x8},
{kWellKnownKeys[common_properties::molStereoCare], 0x10},
}};
// this is stored as bitflags in a byte, so don't exceed 8 entries or we need
// to update the pickle format.
// the properties themselves are stored as std::int16_t
static constexpr std::array<std::pair<std::string_view, std::uint16_t>, 4>
explicitAtomProps{{
{common_properties::molStereoCare, 0x1},
{common_properties::molParity, 0x2},
{common_properties::molInversionFlag, 0x4},
{common_properties::_ChiralityPossible, 0x8},
{kWellKnownKeys[common_properties::molStereoCare], 0x1},
{kWellKnownKeys[common_properties::molParity], 0x2},
{kWellKnownKeys[common_properties::molInversionFlag], 0x4},
{kWellKnownKeys[common_properties::_ChiralityPossible], 0x8},

}};
static constexpr std::array<std::string_view, 2> ignoreAtomProps{
common_properties::molAtomMapNumber,
common_properties::dummyLabel,
kWellKnownKeys[common_properties::molAtomMapNumber],
kWellKnownKeys[common_properties::dummyLabel],
};
std::unordered_set<std::string_view> ignoreBondProps;
PropTracker() {
Expand Down
10 changes: 5 additions & 5 deletions Code/GraphMol/SmilesParse/CXSmilesOps.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2028,10 +2028,10 @@ std::string get_coords_block(const ROMol &mol,
std::string get_atom_props_block(const ROMol &mol,
const std::vector<unsigned int> &atomOrder) {
constexpr std::array<std::string_view, 7> skip = {
common_properties::atomLabel, common_properties::molFileValue,
common_properties::molParity, common_properties::molAtomMapNumber,
common_properties::molStereoCare, common_properties::molRxnExactChange,
common_properties::molInversionFlag};
kWellKnownKeys[common_properties::atomLabel], kWellKnownKeys[common_properties::molFileValue],
kWellKnownKeys[common_properties::molParity], kWellKnownKeys[common_properties::molAtomMapNumber],
kWellKnownKeys[common_properties::molStereoCare], kWellKnownKeys[common_properties::molRxnExactChange],
kWellKnownKeys[common_properties::molInversionFlag]};
std::string res = "";
unsigned int which = 0;
for (auto idx : atomOrder) {
Expand Down Expand Up @@ -2550,7 +2550,7 @@ std::string getCXExtensions(const ROMol &mol, std::uint32_t flags) {
res += ",";
}
res += "$_AV:" +
get_value_block(mol, atomOrder, common_properties::molFileValue) +
get_value_block(mol, atomOrder, keyToString(common_properties::molFileValue)) +
"$";
}
auto radblock = get_radical_block(mol, atomOrder);
Expand Down
2 changes: 1 addition & 1 deletion Code/GraphMol/Subset.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace {
inline void copyComputedProps(const ROMol &src, ROMol &dst) {
dst.updateProps(src);
for (auto &v : dst.getPropList(true, false)) {
if (v != RDKit::detail::computedPropName) {
if (v != keyToString(RDKit::detail::computedPropName)) {
dst.clearProp(v);
}
}
Expand Down
9 changes: 5 additions & 4 deletions Code/GraphMol/Wrap/props.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,11 @@ boost::python::dict GetPropsAsDict(const T &obj, bool includePrivate,
bool includeComputed,
bool autoConvertStrings = true) {
boost::python::dict dict;
const auto &rd_dict = obj.getDict();
auto &rd_dict = obj.getDict();
auto data = rd_dict.getData();

STR_VECT keys = obj.getPropList(includePrivate, includeComputed);
for (const auto &rdvalue : rd_dict) {
for (auto &rdvalue : data) {
if (std::find(keys.begin(), keys.end(), rdvalue.key) == keys.end()) {
continue;
}
Expand Down Expand Up @@ -241,8 +242,8 @@ PyObject *GetPyProp(const RDOb *obj, const std::string &key, bool autoConvert) {
return nullptr;
}
} else {
const auto &rd_dict = obj->getDict();
for (const auto &rdvalue : rd_dict) {
const auto &data = obj->getDict().getData();
for (auto &rdvalue : data) {
if (rdvalue.key == key) {
try {
const auto tag = rdvalue.val.getTag();
Expand Down
8 changes: 4 additions & 4 deletions Code/GraphMol/catch_pickles.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ M END)CTAB"_ctab;

CHECK(pkl.size() > basepkl.size());
// make sure the property names aren't in the pickle
CHECK(pkl.find(common_properties::_MolFileBondType) == std::string::npos);
CHECK(pkl.find(common_properties::_MolFileBondCfg) == std::string::npos);
CHECK(pkl.find(keyToString(common_properties::_MolFileBondType)) == std::string::npos);
CHECK(pkl.find(keyToString(common_properties::_MolFileBondCfg)) == std::string::npos);
// std::cerr << "!!!! " << pkl.size() << " " << basepkl.size() << std::endl;

RWMol mol2(pkl);
Expand Down Expand Up @@ -203,8 +203,8 @@ M END
MolPickler::pickleMol(*mol, pkl);

// make sure the property names aren't in the pickle
CHECK(pkl.find(common_properties::_MolFileBondAttach) == std::string::npos);
CHECK(pkl.find(common_properties::_MolFileBondEndPts) == std::string::npos);
CHECK(pkl.find(keyToString(common_properties::_MolFileBondAttach)) == std::string::npos);
CHECK(pkl.find(keyToString(common_properties::_MolFileBondEndPts)) == std::string::npos);
// std::cerr << "!!!! " << pkl.size() << " " << basepkl.size() << std::endl;

RWMol mol2(pkl);
Expand Down
13 changes: 9 additions & 4 deletions Code/JavaWrappers/Dict.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,17 @@
%include "std_string_view.i"

%ignore RDKit::Dict::Pair;
%ignore RDKit::Dict::InternalPair;
%ignore RDKit::Dict::InternalDataType;
%ignore RDKit::Dict::getInternalData;
%ignore RDKit::PairHolder;
%ignore RDKit::Dict::extend;
%ignore RDKit::Dict::getRDValue;
%ignore RDKit::Dict::begin;
%ignore RDKit::Dict::end;
%ignore RDKit::Dict::const_iterator;
%ignore RDKit::DictKeyIntern;
%ignore RDKit::internKey;
%ignore RDKit::keyToString;
%ignore RDKit::common_properties;
%ignore RDKit::detail::computedPropName;
%include <RDGeneral/DictKeyIntern.h>
%include <RDGeneral/Dict.h>


Loading