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

Add CalculateAverage_ebarlas submission. #22

Merged
merged 4 commits into from
Jan 3, 2024

Conversation

ebarlas
Copy link
Contributor

@ebarlas ebarlas commented Jan 2, 2024

No description provided.

@gunnarmorling
Copy link
Owner

gunnarmorling commented Jan 2, 2024 via email

@ebarlas
Copy link
Contributor Author

ebarlas commented Jan 2, 2024

On a c5.2xlarge AWS EC2 instance (roughly equivalent to target env):

Reference:

real	4m25.991s
user	4m22.901s
sys	0m4.935s

Mine:

real	0m15.288s
user	1m59.042s
sys	0m0.580s

@gunnarmorling
Copy link
Owner

gunnarmorling commented Jan 2, 2024 via email

@ebarlas
Copy link
Contributor Author

ebarlas commented Jan 2, 2024

Thanks! I'm eager to see how it measures up!

I haven't done any micro-optimizations yet. I assume I can simply open subsequent PRs with additional changes?

@gunnarmorling
Copy link
Owner

gunnarmorling commented Jan 2, 2024 via email

…ups. Use ByteBuffer.get(int, byte[], int, int) method. Add more clarifying comments.
@ebarlas
Copy link
Contributor Author

ebarlas commented Jan 3, 2024

Updated exec time after addition optimizations:

real	0m10.828s
user	1m24.306s
sys	0m0.660s

@gunnarmorling
Copy link
Owner

gunnarmorling commented Jan 3, 2024

Hey, could you run mvn license:format to add the missing license header, rebase to latest main and force-push to this PR? Thx!

@ebarlas
Copy link
Contributor Author

ebarlas commented Jan 3, 2024

Sure, looking now...

@ebarlas
Copy link
Contributor Author

ebarlas commented Jan 3, 2024

Done. How does that look now?

@gunnarmorling gunnarmorling merged commit 1b048c8 into gunnarmorling:main Jan 3, 2024
@gunnarmorling
Copy link
Owner

This looks great! 2nd place right now with 00:16.558 after @spullara! Worth trying how yours fares with GraalVM, too, I suppose.

@lobaorn
Copy link

lobaorn commented Jan 3, 2024

Shamelessly sharing this idea for JVM/GC tuning in another PR/discussion? #15 (comment)

@ebarlas
Copy link
Contributor Author

ebarlas commented Jan 3, 2024

@gunnarmorling, silly question: how can I request execution with GraalVM CE? I just re-ran in my test environment and it is indeed noticeably faster there.

I don't see any metadata or env setup indicating GraalVM by @spullara.

@gunnarmorling
Copy link
Owner

@ebarlas, please specify the JDK in the submission PR (see README). Indeed this wasn't the case for @spullara explicitly, but I read between the lines that this is what he tested with so I interpreted this as to use GraalVM. So just create a new PR for if you'd like to do that. Better yet: could you try whether adding sdk use java <your distro> at the beginning of your launch script works? If so, could you create a PR with that and also point that out in the README? Thanks!

Screenshot 2024-01-03 at 17-58-35 Add note about sharing non-Java solutions on GH discussions by rmoff · Pull Request #40 · gunnarmorling_1brc

@ebarlas
Copy link
Contributor Author

ebarlas commented Jan 3, 2024

Okay, thanks!

@ebarlas
Copy link
Contributor Author

ebarlas commented Jan 3, 2024

I created a separate PR with a request to run with GraalVM. Includes SDKMAN command.

#45

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants