Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Speed up element data #912
Speed up element data #912
Changes from all commits
80f24d5
2033eb8
e0c640c
07229c7
9074533
a995ca2
8ab5fab
788f003
17dd9d4
9f829fd
d612446
3bf5dbd
0e1373f
a9782d9
1d182b3
83df612
c611622
55b22ca
e11216c
e7ef771
ff2acbd
9ea9a10
527b6aa
c077f66
7d01bc3
e25ee35
a73bc60
eb87937
f63e2ce
6258a43
3fa06e3
08ff641
25121e3
885e9a9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it work correctly if there is no previous element data?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does, since the original one worked like this as well, it only set a value to CLuaArgument if there was something, otherwise an empty CLuaArgument is == nil
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe just replace the content of
Get
with a call withMapFind
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could but that'd be just a pointless function call, and we can to squeeze out as much performance as we can, and I think this isn't premature optimization(yet)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there will be a real difference
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it converting from SString to const char* and then back?
Or it's optimized by the compiler already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
99% chance optimised away. If in doubt, measure the difference