Skip to content

Inline small methods#1304

Closed
nir-ben-menachem wants to merge 3 commits intoVROOM-Project:masterfrom
nir-ben-menachem:refactor/inline-small-methods
Closed

Inline small methods#1304
nir-ben-menachem wants to merge 3 commits intoVROOM-Project:masterfrom
nir-ben-menachem:refactor/inline-small-methods

Conversation

@nir-ben-menachem
Copy link
Copy Markdown
Contributor

@nir-ben-menachem nir-ben-menachem commented Oct 29, 2025

Issue

Several small struct methods in structures (e.g. BBox) are on hot paths but defined in separate .cpp files,
preventing inlining and impacting performance.
This PR inlines them and also inlines trivial methods (e.g. ComputingTimes) that make their compilation units unnecessary.

Tasks

  • Inline small, frequently called struct methods in structures
  • Remove redundant .cpp implementations
  • Update CHANGELOG.md (remove if irrelevant)
  • review

Copy link
Copy Markdown
Collaborator

@jcoupey jcoupey left a comment

Choose a reason for hiding this comment

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

Thanks for submitting a PR. The BBox::intersects function is indeed used in a hot path in order to filter out some moves from the local search. BBox::extend is probably not critical since it's "only" used upon route update after a move is picked. As for the rest, they're called when setting up the problem or formatting the solution so I would not expect any impact.

Did you have the opportunity to benchmark those changes?

@nir-ben-menachem
Copy link
Copy Markdown
Contributor Author

Thanks for submitting a PR. The BBox::intersects function is indeed used in a hot path in order to filter out some moves from the local search. BBox::extend is probably not critical since it's "only" used upon route update after a move is picked. As for the rest, they're called when setting up the problem or formatting the solution so I would not expect any impact.

Did you have the opportunity to benchmark those changes?

After benchmarking with homberger_1000 and li_lim_1000 and seeing less than 1% percent improvement on average, I'm concluding that these changes are too insignificant to matter, will run these benchmarks beforehand next time I open a PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants