Open NarsGNA opened 1 year ago
Has dynamic dispatch been measured to be a bottleneck?
We haven't run an extensive test to calculate the impact created by dynamic dispatch...
We doubt its gonna be a bottleneck anytime soon... (Our CPU bottlenecks mostly are struct serialization and I/O bottlenecks are the diesel DB calls)
Although it's not a bottleneck rn, I believe it still contributes sufficiently to the CPU cost given how frequently we access Store (this could be considered a low hanging fruit)...
Currently we use a mix of loadtests + tempo to benchmark performance...
Our current plan was to measure the perf difference via the same stack (tempo + loadtests)...
I am interested in collaborating with someone on their project so I can learn the language while they get a sidekick. Let me Know if this is something that could work for you. I Would need you to explain and guide me through my first steps in the language (I already started learning) and set the standard for me in code reviews but after a short while I'm sure I'll become an asset to the project. (I have some experience in Typescript and Java and have learned C a few years ago).
may I give a try with this one?
Hey @fbrv , I believe that now would not be a good time to delve into this since there are other major refactors happening for the storage interface... I'll mark this issue as blocked.
Either you can try to
for the last 2 points feel free to ping me on our discord for any help...
Feature Description
Dynamic dispatch is currently used for accessing the store. While it helps maintain multiple
StorageInterface
implementations, it adds a runtime overhead that needs to be reducedMoving this to generics could be a solution but generics tend to make the code less readable given we already have a lot of generics
[enum_dispatch] is a better solution since we have only a limited set of possible store implementations. The performance benchmarks are already listed in the crate documentation. Currently, the possible
StorageInterface
implementations areStore
andMockDb
. TheAppstate
can be created with the currentStore
Update the Settings to accept the configuration so as to specify the
StorageInterface
to be used.Have you spent some time to check if this feature request has been raised before?
Have you read the Contributing Guidelines?
Are you willing to submit a PR?
No, I don't have time to work on this right now