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.

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

Jump to bottom

Another approach to remove the type instability #105

Merged
merged 1 commit into from
Aug 23, 2022
Merged

Another approach to remove the type instability #105

merged 1 commit into from
Aug 23, 2022

Conversation

sambitdash
Copy link
Owner

To be reviewed along with #104. Pre-create the objects and deepcopy when needed.

@sambitdash
Another approach to remove the type stability
1f30974 
To be reviewed along with #104. Pre-create the objects and `deepcopy` when needed.
@sambitdash sambitdash changed the title Another approach to remove the type stability Another approach to remove the type instability Jul 17, 2022
@sambitdash sambitdash mentioned this pull request Jul 17, 2022
Closed
@sambitdash
Copy link
Owner Author

sambitdash commented Jul 17, 2022 •
edited
Loading

@hhaensel please review this with test cases and see if there is any performance improvement.

@hhaensel
Copy link

Hi, sorry it took me so long to come back to this topic.
It seems that your approach has identical memory usage and speed as mine. I had first tested your approach but forgot to do a deepcopy, so I failed.
I think we should rather go for your approach then.
I'll close #104 then.

@sambitdash sambitdash marked this pull request as ready for review August 23, 2022 12:42
sambitdash
sambitdash commented Aug 23, 2022
View reviewed changes
Copy link
Owner Author

@sambitdash sambitdash left a comment

Choose a reason for hiding this comment

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

Ready to be merged.

@sambitdash sambitdash merged commit 1a2137c into master Aug 23, 2022
@sambitdash sambitdash deleted the sambitdash-patch-1 branch August 24, 2022 10:19
@sambitdash sambitdash mentioned this pull request Aug 25, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sambitdash @hhaensel