-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Correcting an unnecessary function call #83752
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
base: master
Are you sure you want to change the base?
Conversation
…n when creating solid objects
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
If this is performance oriented it needs some performance testing like a use case where it's spending a lot of time and before/after profiles showing it was meaningfully improved. |
|
@kevingranade yes, that's a fair point. I did a run-time test a few minutes ago. Actually, I'm not sure if I did it right, but still the result is there. And the following results came out about creating things in batches of 50. Nothing much has changed for liquid and mixed results, but for completely non-liquid results, the difference is very noticeable. If there is another method like monjo inside VS Code to check performance, then please tell me how to do it. |
|
Hi @Xomikadze92, welcome to CleverRaven! We see that this is your first time commenting here. Check out how development works and be sure to follow the code of conduct! We hope to see you around! |
|
If I'm reading this correctly the overhead we're talking about here is less than one ms per item in the batch, and it only occurs once when a recipe is completed? I'm skeptical that's even noticeable, especially at the end of a many-turn crafting process. |
|
Yes, that's right, the wound is really very small, perhaps it is more noticeable on a weaker PC configuration. But I've learned that in any case, it's better to get rid of unnecessary function calls. Also, this check only works from the crafting menu, but in my opinion it should work the same way when we interrupted crafting and resumed it. So I would like to further optimize the operation of this functionality. |
|
Hi @Xomikadze92, welcome to CleverRaven! We see that this is your first time commenting here. Check out how development works and be sure to follow the code of conduct! We hope to see you around! |
|
What is the performance impact on creating 50 batches of salt using the lye + hydrochloric acid recipe, so ~7000 individual items? |
Summary
Performance "Fixing an unnecessary call to the liquid container inspection function when creating solid objects"
Purpose of change
Remove unnecessary calls to functions for creating objects and others
Describe the solution
When creating items, regardless of the aggregate state, a check (check_eligible_containers_for_crafting) is run to determine whether they will fit or not. At the beginning of the check, items and by-products are "created". And then each unit of production is checked to see if it is liquid. I have implemented a function that immediately checks the final result of crafting, since it is known in advance. And if the result is liquids, then the container check is started in full as before, and if there are no liquids, then the check is not necessary.
Describe alternatives you've considered
It may be worth redoing the check_eligible_containers_for_crafting check to check all the liquids at once, rather than creating them.
Testing
Created item that have only non-liquid results, which have only liquid results and mixed results.
Additional context