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

Move VajramKryonGraph.validateMandatory from VajramKryonGraph to auto-generated code #Vajram #167

Closed
RamAnvesh opened this issue Aug 15, 2023 · 1 comment · Fixed by #327
Assignees
Labels
performance Performance enhancement related size: s Small change
Milestone

Comments

@RamAnvesh
Copy link
Collaborator

Currently VajramKryonGraph.validateMandatory is taking ~10% of the CPU of Kryon.executeMainLogicIfPossible. Most of this time is going in Stream processing and iteration.

I idea is to move this validation into auto-generated vajram models code so that we can omit iteration and stream processing. Instead we can custom generate a validation method which access each input explicitly. This way we can avoid Inputs.getInputValue lookup as well.

@RamAnvesh RamAnvesh added the performance Performance enhancement related label Aug 15, 2023
@RamAnvesh RamAnvesh changed the title Move VajramKryonGraph.validateMandatory from VajramKryonGraph to auto-generated code Move VajramKryonGraph.validateMandatory from VajramKryonGraph to auto-generated code #Vajram Aug 15, 2023
@RamAnvesh RamAnvesh modified the milestone: v9 Feb 2, 2025
@RamAnvesh RamAnvesh linked a pull request Feb 6, 2025 that will close this issue
@RamAnvesh RamAnvesh added the size: s Small change label Feb 6, 2025
@RamAnvesh
Copy link
Collaborator Author

In the new Krystal 9 design, stream has been replaced with for loop and map lookup is completely eliminated as we are using a code generated lambda to get the facet value. So this performance problem should be solved. We are seeing some gains in benchmarks as well. Closing this issue. Fresh profiling will be needed to find new bottlenecks in the new design.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance enhancement related size: s Small change
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants