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

[Enhancement]: Idiomatic restructure #73

Open
1 task done
jonbarrow opened this issue Dec 11, 2024 · 0 comments
Open
1 task done

[Enhancement]: Idiomatic restructure #73

jonbarrow opened this issue Dec 11, 2024 · 0 comments
Labels
approved The topic is approved by a developer enhancement An update to an existing part of the codebase

Comments

@jonbarrow
Copy link
Member

Checked Existing

  • I have checked the repository for duplicate issues.

What enhancement would you like to see?

After much deliberation, it's been decided that we should do a restructure of this project in a more idiomatic way

Prior to now we have been using the v2 release to try and replicate the original NEX/Rendez-Vous implementations/APIs as 1:1 as we possibly can. This was done for a number of reasons, including:

  • It allowed us to move from RE->implementation more quickly (skipping the need to translate our decomp/research notes into idiomatic Go)
  • Matched up more easily with research notes (which is important as we are ran by volunteers and have people coming and going, so being able to get people up to speed and pointed in the right direction for research is important)
  • Was "more accurate" emulation (though really only at the API level I suppose)
  • Anyone who used the original Rendez-Vous library would be able to more easily use ours (though the market for that is arguably non-existent)

It was the opinion of @wolfendale that we should instead focus not on 1:1 implementations, but emulate the intentions of the original authors. Implementing things the way we have has resulted in some footguns and has held us back out of fear of deviating from the original. I believe I was wrong to push back on Wolf's suggestion, he had the right idea from the beginning

I know v2 only recently launched, however I believe it is worth planning a new major release which addresses these issues and redoes the PRUDP implementation from scratch in a more idiomatic way

Any other details to share? (OPTIONAL)

I believe the best way to do this is to continue using the current release like normal, while taking some time to very carefully plan out how we'd want to actually implement things when not constrained to the original API

We should also consider restructuring the repo to be more idiomatic as well. In Go, the "proper" way to do major version releases is to split them into directories. See https://github.com/superwhiskers/crunch for an example of this. For the next release we should probably do the same

We should also take this time to think about some of our implementations. For example we made MutexMap as a way to have a thread-safe map type, however Go already provides this in the form of sync.Map. sync.Map is optimized for multithreaded use under specific circumstances. It's best used in cases where a key is written to once/infrequently, but read MANY times. However it has some drawbacks:

  • If the workload on the map is relatively even (lots of reads, writes, and modifications) then it may not provide better performance than a standard map with a mutex lock (like MutexMap) and may even be worse
  • sync.Map only guarantees safe reads from multiple goroutines. Trying to do a read-modify-store operation is NOT thread safe by itself, as the DATA of the key is not locked. This means there is a race condition where multiple goroutines may try to update the same data at the same time (this will not fail, but it may create bad state)

So we should examine our use cases carefully

Maybe we should create a shared document or diagram or something to help plan this out better?

@jonbarrow jonbarrow added enhancement An update to an existing part of the codebase awaiting-approval Topic has not been approved or denied approved The topic is approved by a developer and removed awaiting-approval Topic has not been approved or denied labels Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved The topic is approved by a developer enhancement An update to an existing part of the codebase
Projects
None yet
Development

No branches or pull requests

1 participant