-
-
Notifications
You must be signed in to change notification settings - Fork 258
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
Compatibility with ring/ring-codec #1091
Comments
You can see an earlier commit in the history of ring-codec in which I made it compatible with babashka. It's a pity that this commit has made it incompatible again: /cc @bsless The paradox with babashka is that lower level code like this is usually slower (or incompatible) with bb than using high level constructs since there is more interpretation overhead. Perhaps we can just maintain a fork of this library or introduce a reader conditional into the main repo, but I'm not sure if the original maintainer is OK with that. |
However, by adding this class, it will make things compatible again. I've tested it locally: (prn (reduce (fn [m param]
(prn param) m)
{} (tokenized "foo&bar&baz" "&")))
I'll make a branch and let CI run it to see what the increased binary size would be. |
Benchmark: (time
(dotimes [_ 100000]
(vec (tokenized "foo&bar&baz" "&"))))
(time
(dotimes [_ 100000]
(str/split "foo&bar&baz" #"&")))
I'm ok with adding the class, since it doesn't take a lot of space, but it does seem to degrade performance somewhat. |
Merged: #1092 |
Thank you! |
Btw, there are some optimizations to be made with respect to interop. The method to invoke can be detected at analysis time rather than run time. This would speed up things considerably for this example, I think. |
See babashka/sci#516 and babashka/sci#644. |
Is your feature request related to a problem? Please describe.
I would love to be able to use ring/ring-codec in holy-lambda-ring-adapter, but unfortunately
java.util.StringTokenizer
is not available inbabashka
.Describe the solution you'd like
May I please propose a PR in which I would add
java.util.StringTokenizer
to a list of supported classes?Describe alternatives you've considered
Making a fork of ring-codec without StringTokenizer.
The text was updated successfully, but these errors were encountered: