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

Fix/clones #5

Merged
merged 13 commits into from
Jul 7, 2024
Merged

Fix/clones #5

merged 13 commits into from
Jul 7, 2024

Conversation

ichorid
Copy link
Contributor

@ichorid ichorid commented Jul 5, 2024

This removes almost all the clones, changes NodeId to size, etc.
As the result, performance improves 4-8 times, and memory usage is reduced 2+ size.

Note that OUT4-based test ("long") was found to be broken: the original input file misplaced some values sometimes.
However, OUT3 (short) test passes now correctly.

@ichorid ichorid requested a review from automainint July 5, 2024 19:00
@automainint automainint self-assigned this Jul 6, 2024
Copy link
Collaborator

@automainint automainint left a comment

Choose a reason for hiding this comment

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

smoke_perf_with_zero test still failing.

@automainint
Copy link
Collaborator

I increased success margin for smoke_perf_with_zero test to 80 seconds for now.

@automainint automainint merged commit 9413d72 into Intersubjective:dev Jul 7, 2024
1 check passed
@ichorid
Copy link
Contributor Author

ichorid commented Jul 7, 2024

smoke_perf_with_zero test still failing.

if fails with a timeout if building with ASSERT enabled. If you disable ASSERT, the test completes in under 5 seconds. We should make smoke_perf_with_zero run with ASSERT = false. This will require either recompiling, or slightly changing the code to be able to turn ASSERT on and off during the runtime.

@ichorid ichorid deleted the fix/clones branch July 7, 2024 20:16
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