Skip to content
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.

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

Remove unnecessary allocation in non-integral reference code #4460

Merged

Conversation

AndrewWestberg
Copy link
Contributor

Description

Temp variables in ref_exp_ function are never used. They are just allocated and cleared. Since this is a recursive function and the reference code serves as a baseline for performance, it should be fixed.

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • Self-reviewed the diff

Copy link
Collaborator

@lehins lehins left a comment

Choose a reason for hiding this comment

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

Nice catch.

Looks like the original version did use those temporary variables, but then usage was removed in e73df76

Note that that there are variables temp_q and temp_r that are used in the second half of the function, but they are defined, initialized and cleared separately, so these two that are being removed are definitely unused

Thank you.

@lehins lehins enabled auto-merge July 2, 2024 19:30
@lehins lehins force-pushed the amw/fix_unnecessary_allocation branch from 3477052 to 29fd7fb Compare July 2, 2024 19:36
@teodanciu teodanciu force-pushed the amw/fix_unnecessary_allocation branch from 29fd7fb to f7d6d52 Compare July 3, 2024 12:22
@lehins lehins merged commit 8cff9cf into IntersectMBO:master Jul 3, 2024
123 of 126 checks passed
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