-
Notifications
You must be signed in to change notification settings - Fork 5
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
using struct instead of sealed class? #93
Comments
Hi @Eraclys, I think I may have started going down that path at one point, but I can't remember why I stopped -- I think the particular microbenchmarks I was doing at the time were actually showing a drop in performance when I switched? It was very early on, though and very likely that whatever I was testing was extremely narrow in scope. So, all that being said, if you are open to throwing together a PR I am definitely interested in taking a look at it. I've been swamped with work the past few months, but I should have some time coming up in the next few weeks to give this project a bit of TLC -- in particular I need to get #86 done so that the library is producing equivalent results to upstream release 4.0.1. Maybe we could get a release out that includes both? Thanks! |
Hi @pocketken, Thank you for your reply. I'll try and have a look whenever I get a chance. |
Hi @Eraclys, did you happen to have a chance to do any playing around with this idea yourself? I'm hoping to start working on the updates required to align with 4.1.0 (#102) at some point in the next week or so. I can't guarantee anything will come out of it, but I am planning on spending a little bit of time investigating this idea as well while I'm in here. If you've done any preliminary work / scoping, let me know! |
Hi @pocketken, I started looking into it but other things came up. |
Yeah; means we'll have to do separate benchmark projects for each and manually compare. |
* fix GH-86: change Vec2d.Intersect to use double instead of float * fix GH-98: add net7.0 TFM * some work on GH-93: convert a few model classes to struct, still lots to do especially to avoid API breakage * replace LeadingZeros polyfill with the actual code from BitConverter.LeadingZeroCount * attempt at optimizing H3Index.LeadingNonZeroDirection * optimize H3Index.RotatePentagonClockwise and H3Index.RotatePentagonCounterClockwise * update benchmarks to focus on comparison between 4.0.0 and eventual 4.1.0 release
Haven't forgotten about this @Eraclys have just been swamped. Actually I need a new word for swamped, as it does not adequately describe the past few months at all... anyway! I will finally get back to looking at this in the next couple of weeks now that things are finally calming down a bit. |
Hello @pocketken ,
Just wondering if you have considered using struct instead of a sealed class for H3Index to avoid memory allocations on the heap and make better use of processor caching?
I've been considering making a PR but you may already have investigated the subject?
Thanks
The text was updated successfully, but these errors were encountered: