search

schrodinger / coordgenlibs

Schrodinger-developed 2D Coordinate Generation
BSD 3-Clause "New" or "Revised" License
42 stars 28 forks source link

Reduce number of arguments passed by value #36

Closed ricrogz closed 5 years ago

ricrogz commented 5 years ago

I noticed that we are passing a fair amount of function arguments by value (mostly sketcherMinimizerPointF, vectors of pointers, and a few maps), which introduces a great deal of copy operations that we could avoid.

I made most of these arguments passed by const ref, added a few std::moves, and deleted some blocks of commented code I noticed. clang-format also made some minor changes.

Checked that it builds, passes the test included in #35, and that RDKit and mmshare have no problem building after these changes.

ricrogz commented 5 years ago

I ran the tests in RDKit and mmshare: some tests in mmshare failed (no big deal, seems like some small differences in SMARTS strings).

I checked it, and it seems these problems were caused by recent fixes in RDKit rather than the changes here, as the problems disappeared when I used the same RDKit commit that we are currently using in mmshare.

d-b-w commented 5 years ago

Thanks, Ricardo. I'll merge this once Nic has had a chance to take a look.

ricrogz commented 5 years ago

Thanks, Ricardo. I'll merge this once Nic has had a chance to take a look.

Sounds good.