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

Rewrite Project #599

Closed
wants to merge 11 commits into from
Closed

Rewrite Project #599

wants to merge 11 commits into from

Conversation

rj00a
Copy link
Member

@rj00a rj00a commented Feb 3, 2024

Rewrite the project from the ground up. Closes #596

Steps

  • Update data extractor to the latest version and fix known issues.
  • Fix issues in java_string.
  • Rework interface of valence_nbt to support redesign of valence_protocol.
  • Remove valence_generated.
    • Computing everything statically doesn't work in a world where more and more registries are networked.
    • Compile times were too long. We can generate stuff from the JSON at runtime if we need to.
  • Redesign valence_protocol and update to 1.20.4.
    • Reorganize packets into a single module. (Minecraft shares some packets between protocol stages now, so this accommodates for that).
    • Introduce McRead and McWrite traits similar to PacketByteBuf in vanilla.
    • Remove bespoke derive macro for "encodable" and "decodable" types and write all the packet impls out manually.
    • Remove dependence on valence_generated, bevy_ecs, valence_math.
    • Consider merging valence_text and valence_ident into valence_protocol since we don't have valence_generated anymore.
  • Update the packet inspector and get it to a usable state.
    • Create "packet registry" in valence_protocol so the inspector can access all packets programmatically.
  • Rewrite main server crates.
    • Replace bevy_ecs with evenio. Use event driven design described in Rewrite the project with evenio #596.
    • Use monoio + ureq on Linux and MacOS, tokio everywhere else.
    • Handle switching between configuration phase and play phase.
    • Get basic chunk stuff working at least.
    • Combine the miscellaneous crates together like weather, player_list, boss_bar, registry, etc.
  • Pick a linear algebra crate (glam, vek, ultraviolet, something else?)
  • Port some of the examples over.
  • Update valence_anvil (or leave that for follow up?)
  • Update CI scripts.

rj00a and others added 6 commits February 16, 2024 05:32
@andrewgazelka
Copy link

andrewgazelka commented Mar 17, 2024

Given that you are making a bunch of changes and performance is important, have you considered adding benching to CI? I feel like I have seen Rust projects that have good performance regression detection in CI, but I might need to do a bit of searching.

This would be nice because performance would be repeatable (hopefully!) unlike

if I have singleplayer Minecraft open on my machine at the same time I'm running bench_players, the performance actually improves.

#323

@rj00a
Copy link
Member Author

rj00a commented Mar 22, 2024

@andrewgazelka I'm open to the idea, but it seems like a lot of work to do it correctly. For starters we would need dedicated CI runners and a decent suite of benches. Definitely something to think about after the rewrite is finished.

@A-z-z-y
Copy link

A-z-z-y commented Mar 22, 2024

The criterion crate sounds like it may be useful here, unless ya'll have your own favorite (micro)benchmarking tools.

@andrewgazelka
Copy link

@andrewgazelka I'm open to the idea, but it seems like a lot of work to do it correctly. For starters we would need dedicated CI runners and a decent suite of benches. Definitely something to think about after the rewrite is finished.

That makes sense. Why would you need dedicated CI runners, though? Do you think performance would not be predictable using the default CI runners, which run in a VM on the same machine as other runners?

@A-z-z-y
Copy link

A-z-z-y commented Mar 23, 2024

If consistent environments are available (e.g. on your own machine), it may be worth it to use a microbenchmarking crate like criterion to check for performance regressions.

@rj00a
Copy link
Member Author

rj00a commented Mar 25, 2024

Do you think performance would not be predictable using the default CI runners, which run in a VM on the same machine as other runners?

That's right, it's too noisy from what I've heard.

@A-z-z-y I'm quite fond of divan nowadays :)

@splurf
Copy link

splurf commented Apr 3, 2024

The crates glam and nalgebra are both well-maintained and highly optimized. There's a benchmark that compares a bunch of popular lin alg crates but it's over 3 years old.

@rj00a rj00a mentioned this pull request Jun 3, 2024
13 tasks
@rj00a
Copy link
Member Author

rj00a commented Jun 3, 2024

Closing this since I'm not happy with the direction this was going and it's too much to do in one PR anyway.

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