perf: skip unique-rank atoms in iterateCIPRanks inner loop#12
Open
perf: skip unique-rank atoms in iterateCIPRanks inner loop#12
Conversation
Atoms that already have a unique rank cannot change relative position in subsequent iterations, so we skip the neighbor-rank accumulation for them. A dynamic_bitset tracks which atoms are in tied segments (from findSegmentsToResort), and only those atoms participate in the inner loop.
== /workdir/tmp/bench_results/master.txt → /workdir/tmp/bench_results/opt-cip-ranks.txt == Benchmark Old (mean±sd) New (mean±sd) Δ% Speed Sig(p) ------------------------------------------------------------------------------------------------------------------------------- bench_common::nth_random 0.544 ns ± 0.000696 ns 0.546 ns ± 0.00241 ns +0.4% 1.00× slower ns p=0.138 Chirality::findPotentialStereo 624 us ± 32.1 us 614 us ± 46 us -1.5% 1.02× faster ns p=0.767 CIPLabeler::assignCIPLabels 268 ms ± 774 us 266 ms ± 964 us -0.7% 1.01× faster below p=0.0142 Descriptors::calcNumBridgeheadAtoms 1.06 us ± 7.95 ns 1.07 us ± 19.2 ns +0.9% 1.01× slower ns p=0.402 Descriptors::calcNumSpiroAtoms 1.26 us ± 7.22 ns 1.28 us ± 97.3 ns +1.9% 1.02× slower ns p=0.669 InchiToInchiKey 39.2 us ± 5.36 us 38.8 us ± 5.92 us -0.8% 1.01× faster ns p=0.943 InchiToMol 4.21 ms ± 85 us 4.21 ms ± 30.4 us -0.1% 1.00× faster ns p=0.942 memory pressure test 9.36 us ± 822 ns 8.96 us ± 213 ns -4.3% 1.04× faster ns p=0.416 MolOps::addHs 431 us ± 24.3 us 432 us ± 2.69 us +0.3% 1.00× slower ns p=0.938 MolOps::assignStereochemistry legacy=false 759 us ± 36.4 us 754 us ± 45.2 us -0.7% 1.01× faster ns p=0.876 MolOps::assignStereochemistry legacy=true 470 us ± 1.93 us 496 us ± 25.5 us +5.7% 1.06× slower ns p=0.0716 MolOps::FindSSR 190 us ± 15.4 us 176 us ± 6.97 us -7.5% 1.08× faster ns p=0.142 MolOps::getMolFrags 1.08 ms ± 28.4 us 1.08 ms ± 26.2 us +0.0% 1.00× slower ns p=0.99 MolPickler::molFromPickle 159 us ± 784 ns 159 us ± 1.1 us +0.3% 1.00× slower ns p=0.601 MolPickler::pickleMol 106 us ± 7.08 us 111 us ± 3.44 us +4.7% 1.05× slower ns p=0.276 MolToCXSmiles 1.36 ms ± 21.7 us 1.28 ms ± 58.3 us -5.6% 1.06× faster sig p=0.0331 MolToInchi 2.06 ms ± 62.9 us 1.99 ms ± 13.8 us -3.3% 1.03× faster ns p=0.066 MolToSmiles 1.04 ms ± 84.1 us 1.01 ms ± 55.8 us -3.1% 1.03× faster ns p=0.585 MorganFingerprints::getFingerprint 193 us ± 11.6 us 185 us ± 821 ns -4.2% 1.04× faster ns p=0.232 ROMol copy constructor 44 us ± 1.84 us 44.5 us ± 271 ns +1.1% 1.01× slower ns p=0.655 ROMol destructor 17.5 us ± 218 ns 18.7 us ± 1.89 us +6.8% 1.07× slower ns p=0.275 ROMol::getNumHeavyAtoms 126 ns ± 34 ns 107 ns ± 3.79 ns -15.2% 1.18× faster ns p=0.334 ROMol::GetSubstructMatch 34.1 us ± 1.88 us 36.1 us ± 5.03 us +5.9% 1.06× slower ns p=0.519 ROMol::GetSubstructMatch patty 1.14 ms ± 31.2 us 1.12 ms ± 5.56 us -2.1% 1.02× faster ns p=0.189 ROMol::GetSubstructMatch RLewis 7.08 ms ± 75.5 us 7.06 ms ± 65.5 us -0.3% 1.00× faster ns p=0.736 SmilesToMol 1.24 ms ± 17.1 us 1.2 ms ± 39 us -3.1% 1.03× faster ns p=0.118 Summary: sig=1, below=1 (stat-sig but < threshold), ns=24, missing=0, new_only=0
== tmp/bench_results/master.txt → tmp/bench_results/opt-cip-ranks.txt == Benchmark Old (mean±sd) New (mean±sd) Δ% Speed Sig(p) ------------------------------------------------------------------------------------------------------------------------- bench_common::nth_random 1.24 ns ± 0.0173 ns 1.76 ns ± 0.612 ns +42.1% 1.42× slower ns p=0.0561 Chirality::findPotentialStereo 1.71 ms ± 262 us 1.61 ms ± 207 us -5.9% 1.06× faster ns p=0.5 CIPLabeler::assignCIPLabels 589 ms ± 3.85 ms 605 ms ± 5.79 ms +2.8% 1.03× slower sig p=1.12e-06 Descriptors::calcNumBridgeheadAtoms 5.3 us ± 1.54 us 3.95 us ± 62.9 ns -25.4% 1.34× faster ns p=0.0518 Descriptors::calcNumSpiroAtoms 6.22 us ± 2.27 us 5.62 us ± 1.88 us -9.6% 1.11× faster ns p=0.65 InchiToInchiKey 57.1 us ± 11.2 us 66.9 us ± 20.3 us +17.2% 1.17× slower ns p=0.384 InchiToMol 11.1 ms ± 633 us 11.8 ms ± 955 us +6.0% 1.06× slower ns p=0.225 memory pressure test 24.9 us ± 7.97 us 25.3 us ± 7.77 us +1.7% 1.02× slower ns p=0.933 MolOps::addHs 849 us ± 178 us 939 us ± 99.3 us +10.6% 1.11× slower ns p=0.324 MolOps::assignStereochemistry legacy=false 2.35 ms ± 221 us 2.02 ms ± 201 us -14.3% 1.17× faster sig p=0.0165 MolOps::assignStereochemistry legacy=true 1.42 ms ± 48.3 us 1.25 ms ± 179 us -12.1% 1.14× faster ns p=0.0604 MolOps::FindSSR 495 us ± 31.4 us 415 us ± 76.4 us -16.0% 1.19× faster sig p=0.0322 MolOps::getMolFrags 2.76 ms ± 311 us 2.68 ms ± 362 us -2.6% 1.03× faster ns p=0.733 MolPickler::molFromPickle 378 us ± 62.9 us 351 us ± 70.7 us -7.3% 1.08× faster ns p=0.516 MolPickler::pickleMol 370 us ± 62.2 us 330 us ± 66.4 us -10.8% 1.12× faster ns p=0.327 MolToCXSmiles 2.8 ms ± 374 us 2.71 ms ± 310 us -3.1% 1.03× faster ns p=0.685 MolToInchi 6.12 ms ± 742 us 6.46 ms ± 488 us +5.5% 1.06× slower ns p=0.413 MolToSmiles 2.16 ms ± 201 us 2.07 ms ± 211 us -4.1% 1.04× faster ns p=0.5 MorganFingerprints::getFingerprint 838 us ± 81.8 us 769 us ± 148 us -8.2% 1.09× faster ns p=0.362 ROMol copy constructor 206 us ± 25.5 us 155 us ± 36 us -24.7% 1.33× faster sig p=0.00992 ROMol destructor 72 us ± 17.8 us 67.5 us ± 17.6 us -6.3% 1.07× faster ns p=0.685 ROMol::getNumHeavyAtoms 388 ns ± 92.8 ns 354 ns ± 72.7 ns -8.8% 1.10× faster ns p=0.519 ROMol::GetSubstructMatch 143 us ± 37 us 123 us ± 37.8 us -13.8% 1.16× faster ns p=0.434 ROMol::GetSubstructMatch patty 3.9 ms ± 533 us 3.96 ms ± 705 us +1.5% 1.02× slower ns p=0.888 ROMol::GetSubstructMatch RLewis 24.9 ms ± 823 us 24.4 ms ± 909 us -2.4% 1.02× faster ns p=0.316 SmilesToMol 3.61 ms ± 321 us 3.48 ms ± 400 us -3.5% 1.04× faster ns p=0.578 Summary: sig=4, below=0 (stat-sig but < threshold), ns=22, missing=0, new_only=0
== /home/ubuntu/rdkits/tmp/bench_results/master.txt → /home/ubuntu/rdkits/tmp/bench_results/opt-cip-ranks.txt == Benchmark Old (mean±sd) New (mean±sd) Δ% CV Speed Sig(p) ------------------------------------------------------------------------------------------------------------------------------ bench_common::nth_random 1.23 ns ± 0.00269 ns 1.72 ns ± 0.516 ns +39.4% 30.0% 1.39× slower ns p=0.103 Chirality::findPotentialStereo 1.56 ms ± 125 us 1.61 ms ± 99.7 us +3.1% 8.0% 1.03× slower ns p=0.513 CIPLabeler::assignCIPLabels 524 ms ± 3.71 ms 524 ms ± 4.68 ms +0.0% 0.9% 1.00× slower ns p=0.943 Descriptors::calcNumBridgeheadAtoms 4.38 us ± 1.11 us 5.01 us ± 989 ns +14.4% 25.3% 1.14× slower ns p=0.37 Descriptors::calcNumSpiroAtoms 4.98 us ± 726 ns 4.92 us ± 700 ns -1.2% 14.6% 1.01× faster ns p=0.9 InchiToInchiKey 55.3 us ± 11 us 49.7 us ± 6.52 us -10.0% 19.8% 1.11× faster ns p=0.365 InchiToMol 10.6 ms ± 207 us 10.4 ms ± 245 us -2.4% 2.4% 1.02× faster ns p=0.112 memory pressure test 22.5 us ± 5.22 us 21.7 us ± 3.96 us -3.5% 23.2% 1.04× faster ns p=0.794 MolOps::addHs 800 us ± 43.1 us 806 us ± 44.8 us +0.8% 5.6% 1.01× slower ns p=0.829 MolOps::assignStereochemistry legacy=false 1.95 ms ± 151 us 2 ms ± 54 us +2.8% 7.8% 1.03× slower ns p=0.476 MolOps::assignStereochemistry legacy=true 1.13 ms ± 86.2 us 1.09 ms ± 55.6 us -3.6% 7.6% 1.04× faster ns p=0.408 MolOps::FindSSR 357 us ± 5.5 us 373 us ± 20 us +4.3% 5.4% 1.04× slower ns p=0.163 MolOps::getMolFrags 2.16 ms ± 155 us 2.32 ms ± 70.1 us +7.5% 7.2% 1.07× slower ns p=0.0807 MolPickler::molFromPickle 312 us ± 37.8 us 311 us ± 27.7 us -0.5% 12.1% 1.01× faster ns p=0.938 MolPickler::pickleMol 273 us ± 36.7 us 307 us ± 29.2 us +12.5% 13.5% 1.13× slower ns p=0.144 MolToCXSmiles 2.47 ms ± 90.6 us 2.52 ms ± 129 us +1.7% 5.1% 1.02× slower ns p=0.559 MolToInchi 5.82 ms ± 181 us 5.62 ms ± 235 us -3.4% 4.2% 1.04× faster ns p=0.172 MolToSmiles 1.99 ms ± 80.1 us 1.93 ms ± 120 us -3.1% 6.2% 1.03× faster ns p=0.373 MorganFingerprints::getFingerprint 670 us ± 20.7 us 617 us ± 55.1 us -8.0% 8.9% 1.09× faster ns p=0.0962 ROMol copy constructor 159 us ± 22.6 us 160 us ± 28.1 us +0.3% 17.6% 1.00× slower ns p=0.973 ROMol destructor 64.3 us ± 6.59 us 62.1 us ± 5.09 us -3.4% 10.3% 1.03× faster ns p=0.579 ROMol::getNumHeavyAtoms 338 ns ± 40.2 ns 330 ns ± 16.4 ns -2.3% 11.9% 1.02× faster ns p=0.702 ROMol::GetSubstructMatch 114 us ± 22 us 108 us ± 16.6 us -5.2% 19.3% 1.06× faster ns p=0.641 ROMol::GetSubstructMatch patty 3.31 ms ± 212 us 3.42 ms ± 258 us +3.1% 7.5% 1.03× slower ns p=0.514 ROMol::GetSubstructMatch RLewis 21.3 ms ± 450 us 20.9 ms ± 640 us -1.6% 3.1% 1.02× faster ns p=0.356 SmilesToMol 2.95 ms ± 171 us 2.81 ms ± 130 us -4.8% 5.8% 1.05× faster ns p=0.183 Summary: sig=0, below=0 (stat-sig but < threshold), ns=26, missing=0, new_only=0 Bonferroni-corrected α = 0.001923 (26 tests, family α = 0.05)
== /home/ubuntu/rdkits/tmp/bench_results/master.txt → /home/ubuntu/rdkits/tmp/bench_results/opt-cip-ranks.txt == Benchmark Old (mean±sd) New (mean±sd) Δ% CV Speed Sig(p) ----------------------------------------------------------------------------------------------------------------------------- bench_common::nth_random 1.24 ns ± 0.00962 ns 1.55 ns ± 0.5 ns +25.2% 32.3% 1.25× slower ns p=0.236 Chirality::findPotentialStereo 1.62 ms ± 130 us 1.62 ms ± 100 us +0.1% 8.0% 1.00× slower ns p=0.982 CIPLabeler::assignCIPLabels 595 ms ± 28.7 ms 556 ms ± 29.2 ms -6.6% 5.3% 1.07× faster ns p=0.0653 Descriptors::calcNumBridgeheadAtoms 4.57 us ± 1.46 us 4.53 us ± 1.38 us -0.9% 32.0% 1.01× faster ns p=0.965 Descriptors::calcNumSpiroAtoms 5.18 us ± 818 ns 5 us ± 1.18 us -3.5% 23.5% 1.04× faster ns p=0.784 InchiToInchiKey 59.2 us ± 9.23 us 52 us ± 2.05 us -12.2% 15.6% 1.14× faster ns p=0.156 InchiToMol 10.7 ms ± 150 us 10.5 ms ± 113 us -2.6% 1.4% 1.03× faster ns p=0.0126 memory pressure test 17.7 us ± 3.07 us 13.8 us ± 2.55 us -21.8% 18.5% 1.28× faster ns p=0.0635 MolOps::addHs 850 us ± 37.7 us 785 us ± 42.2 us -7.7% 5.4% 1.08× faster ns p=0.033 MolOps::assignStereochemistry legacy=false 1.91 ms ± 92.2 us 1.9 ms ± 122 us -0.3% 6.4% 1.00× faster ns p=0.933 MolOps::assignStereochemistry legacy=true 1.18 ms ± 35.2 us 1.19 ms ± 70 us +0.4% 5.9% 1.00× slower ns p=0.902 MolOps::FindSSR 391 us ± 31.6 us 414 us ± 34.1 us +5.8% 8.2% 1.06× slower ns p=0.308 MolOps::getMolFrags 2.35 ms ± 120 us 2.35 ms ± 118 us +0.1% 5.1% 1.00× slower ns p=0.985 MolPickler::molFromPickle 329 us ± 26.5 us 307 us ± 24.7 us -6.6% 8.1% 1.07× faster ns p=0.218 MolPickler::pickleMol 292 us ± 40 us 291 us ± 34.1 us -0.1% 13.7% 1.00× faster ns p=0.986 MolToCXSmiles 2.6 ms ± 119 us 2.54 ms ± 143 us -2.4% 5.6% 1.02× faster ns p=0.478 MolToInchi 6.33 ms ± 218 us 6.04 ms ± 209 us -4.6% 3.5% 1.05× faster ns p=0.0637 MolToSmiles 2.09 ms ± 119 us 1.91 ms ± 77.7 us -8.5% 5.7% 1.09× faster ns p=0.0275 MorganFingerprints::getFingerprint 626 us ± 44.4 us 653 us ± 32.5 us +4.3% 7.1% 1.04× slower ns p=0.31 ROMol copy constructor 157 us ± 20.5 us 162 us ± 15.9 us +3.2% 13.1% 1.03× slower ns p=0.682 ROMol destructor 68.5 us ± 9.76 us 62.9 us ± 9.1 us -8.2% 14.5% 1.09× faster ns p=0.374 ROMol::getNumHeavyAtoms 369 ns ± 67.8 ns 326 ns ± 3.98 ns -11.8% 18.4% 1.13× faster ns p=0.223 ROMol::GetSubstructMatch 142 us ± 22.9 us 133 us ± 24.1 us -6.8% 18.1% 1.07× faster ns p=0.531 ROMol::GetSubstructMatch patty 3.73 ms ± 132 us 3.7 ms ± 212 us -0.6% 5.7% 1.01× faster ns p=0.838 ROMol::GetSubstructMatch RLewis 22.4 ms ± 279 us 22.2 ms ± 415 us -1.0% 1.9% 1.01× faster ns p=0.337 SmilesToMol 2.98 ms ± 226 us 2.96 ms ± 226 us -0.6% 7.6% 1.01× faster ns p=0.905 Summary: sig=0, below=0 (stat-sig but < threshold), ns=26, missing=0, new_only=0 Bonferroni-corrected α = 0.001923 (26 tests, family α = 0.05)
== /home/ubuntu/rdkits/tmp/bench_results/master.txt → /home/ubuntu/rdkits/tmp/bench_results/opt-cip-ranks.txt == Benchmark Old (mean±sd) New (mean±sd) Δ% CV Speed Sig(p) -------------------------------------------------------------------------------------------------------------------------- bench_common::nth_random 1.41 ns ± 0.115 ns 1.3 ns ± 0.131 ns -7.6% 10.1% 1.08× faster ns p=0.0198 Chirality::findPotentialStereo 1.59 ms ± 57 us 1.58 ms ± 68.7 us -0.4% 4.3% 1.00× faster ns p=0.798 CIPLabeler::assignCIPLabels 583 ms ± 32.5 ms 574 ms ± 33.6 ms -1.4% 5.8% 1.01× faster ns p=0.483 Descriptors::calcNumBridgeheadAtoms 4.51 us ± 452 ns 4.28 us ± 453 ns -5.1% 10.6% 1.05× faster ns p=0.16 Descriptors::calcNumSpiroAtoms 5.28 us ± 486 ns 5.13 us ± 505 ns -2.7% 9.8% 1.03× faster ns p=0.423 InchiToInchiKey 50 us ± 5.41 us 50.7 us ± 4.04 us +1.3% 10.8% 1.01× slower ns p=0.713 InchiToMol 10.5 ms ± 189 us 10.5 ms ± 218 us -0.0% 2.1% 1.00× faster ns p=0.989 memory pressure test 17.8 us ± 3.76 us 16.4 us ± 2.89 us -7.7% 21.1% 1.08× faster ns p=0.257 MolOps::addHs 772 us ± 40 us 797 us ± 22.6 us +3.3% 5.2% 1.03× slower ns p=0.0363 MolOps::assignStereochemistry legacy=false 1.93 ms ± 75 us 1.92 ms ± 54.6 us -0.3% 3.9% 1.00× faster ns p=0.829 MolOps::assignStereochemistry legacy=true 1.11 ms ± 38.7 us 1.08 ms ± 35.9 us -2.4% 3.5% 1.02× faster ns p=0.0573 MolOps::FindSSR 376 us ± 19.1 us 377 us ± 23.3 us +0.1% 6.2% 1.00× slower ns p=0.955 MolOps::getMolFrags 2.29 ms ± 93.1 us 2.29 ms ± 98.4 us -0.2% 4.3% 1.00× faster ns p=0.873 MolPickler::molFromPickle 301 us ± 20.2 us 301 us ± 17 us -0.1% 6.7% 1.00× faster ns p=0.953 MolPickler::pickleMol 281 us ± 16.4 us 276 us ± 18 us -1.7% 6.5% 1.02× faster ns p=0.434 MolToCXSmiles 2.57 ms ± 118 us 2.57 ms ± 88 us +0.3% 4.6% 1.00× slower ns p=0.858 MolToInchi 6.1 ms ± 113 us 6.13 ms ± 97.5 us +0.5% 1.9% 1.01× slower ns p=0.391 MolToSmiles 1.98 ms ± 116 us 1.99 ms ± 65.8 us +0.1% 5.8% 1.00× slower ns p=0.95 MorganFingerprints::getFingerprint 613 us ± 22.3 us 625 us ± 23.6 us +1.9% 3.8% 1.02× slower ns p=0.155 ROMol copy constructor 148 us ± 10.2 us 148 us ± 9.26 us -0.0% 6.9% 1.00× faster ns p=1 ROMol destructor 61.7 us ± 4.23 us 62.8 us ± 3.36 us +1.8% 6.9% 1.02× slower ns p=0.412 ROMol::getNumHeavyAtoms 363 ns ± 31.8 ns 348 ns ± 27.5 ns -4.1% 8.8% 1.04× faster ns p=0.164 ROMol::GetSubstructMatch 115 us ± 10.6 us 110 us ± 8.51 us -4.5% 9.2% 1.05× faster ns p=0.14 ROMol::GetSubstructMatch patty 3.62 ms ± 60.1 us 3.65 ms ± 91.9 us +0.8% 2.5% 1.01× slower ns p=0.279 ROMol::GetSubstructMatch RLewis 22.2 ms ± 343 us 22 ms ± 335 us -1.1% 1.5% 1.01× faster ns p=0.0436 SmilesToMol 2.99 ms ± 80.9 us 2.97 ms ± 91.4 us -0.7% 3.1% 1.01× faster ns p=0.492 Summary: sig=0, below=0 (stat-sig but < threshold), ns=26, missing=0, new_only=0 Bonferroni-corrected α = 0.001923 (26 tests, family α = 0.05)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
summary by opus-4.6 via opencode:
Skip atoms that already have a unique rank in the
iterateCIPRanksinner loop. Atoms with unique ranks cannot change relative position in subsequent iterations, so the neighbor-rank accumulation is wasted work for them.A
boost::dynamic_bitsettracks which atoms are in tied segments (fromfindSegmentsToResort), and only those atoms participate in the inner loop.Benchmark (16 rounds × 800 samples, interleaved, Bonferroni-corrected α=0.002):
No significant results (0/26).
Benchmark Results (vs master)
Summary: sig=0, below=0 (stat-sig but < threshold), ns=26, missing=0, new_only=0
Bonferroni-corrected α = 0.001923 (26 tests, family α = 0.05)