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
Add benchmarks comparing performance of Table operations 'vectorized' in Java vs performed in Enso #7270
Add benchmarks comparing performance of Table operations 'vectorized' in Java vs performed in Enso #7270
Changes from 30 commits
1be9e55
b6cf40e
6e5d718
aa829f6
b5781eb
23f5396
81a33a1
25d465a
d93ff75
6f46c42
fc305f7
1b2fda6
4641920
81e0db4
171cdb2
b6fe3b7
1561444
2cb4188
a9e0825
ba375fc
b642b1c
d8ad9ca
0e5d8a7
d650516
d3224f7
863e486
f5d8ae1
a09f73f
be66a76
863903a
e4853f9
cd0a984
d10009a
06d17ba
f1bf33e
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.
I do remember when
fromItems
introducedList<Value>
... deciding whetherList<Value>
is needed or whetherList<Object>
is enough is a delicate dance balancing GraalVM Host Interop, GraalVM version, Enso implementation of interop and needs of the libraries.In any case, I believe the #1 thing we need is to get stable, independent benchmarks being executed daily by the CI. Without it we will just shift the code back and forth without being sure the system is improved.
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.
For now I will keep it as is, but I will be adding tickets to revisit this.
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.
At this point, I don't think we should add this to the core
Column
type.It can be a static method anywhere from my read so I suggest it goes into the exploratory helper Java.
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 wanted to keep it in std-lib, because I actually plan to use it in #6111 - there when we know the expected value type, if the value type is not
Mixed
norDate
(etc.), we can guarantee no dates will come in - and so we do not need the conversion to happen. This will make the map operation and construction of columns like integer column much faster.I can make it a static, but since I plan to use it in the main codebase soon, I did not want to move it to the helpers - since I'd have to move it back again when implementing #6111
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.
Fair - felt for an experiment we shouldn't be changing the core but happy given next step will be to change it.