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

Log & tracing improvements #2753

Draft
wants to merge 67 commits into
base: next
Choose a base branch
from
Draft

Commits on Jan 22, 2024

  1. Remove temporary Vec usage in the http module (serenity-rs#2624, s…

    …erenity-rs#2646)
    
    This avoids having to allocate to store fixed length (replaced with normal
    array) or fixed capacity (replaced with `ArrayVec`) collections as vectors for
    the purposes of putting them through the `Request` plumbing.
    
    Slight behavioral change - before, setting `params` to `Some(vec![])`
    would still append a question mark to the end of the url. Now, we check
    if the params array `is_empty` instead of `is_some`, so the question
    mark won't be appended if the params list is empty.
    
    Co-authored-by: Michael Krasnitski <42564254+mkrasnitski@users.noreply.github.com>
    2 people authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    40d1815 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    e63dee2 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    40a787e View commit details
    Browse the repository at this point in the history
  4. Remove *_arc methods (serenity-rs#2654)

    These are unnecessary. Accepting `impl Into<Arc<T>>` allows passing either `T` or `Arc<T>`.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    7fad87a View commit details
    Browse the repository at this point in the history
  5. Configuration menu
    Copy the full SHA
    3d7e140 View commit details
    Browse the repository at this point in the history
  6. Put Message::thread behind a Box (serenity-rs#2658)

    This trades a heap allocation for messages sent along with thread
    creation for `Message`'s inline size dropping from 1176 bytes to 760
    bytes,
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    f28ea48 View commit details
    Browse the repository at this point in the history
  7. Replace Vec and String with FixedArray and FixedString for al…

    …l models (serenity-rs#2656)
    
    This shrinks type sizes by a lot; however, it makes the user experience slightly
    different:
    
    - `FixedString` must be converted to String with `.into()` or `.into_string()`
      before it can be pushed to, but dereferences to `&str` as is.
    - `FixedArray` must be converted to `Vec` with `.into()` or `.into_vec()`
      before it can be pushed to, but dereferences to `&[T]` as is.
    
    The crate of these types is currently a Git dependency, but this is fine for
    the `next` branch. It needs some basic testing, which Serenity is perfect for,
    before a release will be made to crates.io.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    b8fcea2 View commit details
    Browse the repository at this point in the history
  8. Fix truncation when using FixedString<u8> where multibyte character…

    …s push over the limit (serenity-rs#2660)
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    987ab43 View commit details
    Browse the repository at this point in the history
  9. Configuration menu
    Copy the full SHA
    226d853 View commit details
    Browse the repository at this point in the history
  10. Switch to i64 in methods associated with CreateCommandOption (ser…

    …enity-rs#2668)
    
    This commit:
    
    - switches from `u64` to `i64` in `CreateCommandOption::min_int_value` and
    `CreateCommandOption::max_int_value` to accommodate negative integers in
    Discord's integer range (between -2^53 and 2^53). Values outside this
    range will cause Discord's API to return an error.
    - switches from `i32` to `i64` in `CreateCommandOption::add_int_choice` and
    `CreateCommandOption::add_int_choice_localized` to accommodate Discord's
    complete integer range (between -2^53 and 2^53). Values outside this
    range will cause Discord's API to return an error.
    ARandomDev99 authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    1fbf27d View commit details
    Browse the repository at this point in the history
  11. Get rid of the duplicate users cache (serenity-rs#2662)

    This cache was just duplicating information already present in `Guild::members`
    and therefore should be removed. 
    
    This saves around 700 MBs for my bot (pre-`FixedString`).
    
    This has to refactor `utils::content_safe` to always take a `Guild` instead
    of`Cache`, but in practice it was mostly pulling from the guild cache anyway
    and this means it is more likely to respect nicknames and other information,
    while losing the ability to clean mentions from DMs, which do not matter.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    371e77c View commit details
    Browse the repository at this point in the history
  12. Change Embed::fields to use FixedArray (serenity-rs#2674)

    `Embed::fields` previously had to stay as a `Vec` due to `CreateEmbed` wrapping
    around it, but by implementing `Serialize` manually we can overwrite the
    `Embed::fields` with a normal `Vec`, for a small performance hit on
    serialization while saving some space for all stored `Embed`s.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    8c97364 View commit details
    Browse the repository at this point in the history
  13. Use FixedArray and FixedString in model enums (serenity-rs#2675)

    Simply missed these when finding and replacing.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    4a159a3 View commit details
    Browse the repository at this point in the history
  14. Bitpack boolean fields using fancy proc macro (serenity-rs#2673)

    This uses the `bool_to_bitflags` macro to remove boolean (and optional boolean)
    fields from structs and pack them into a bitflags invocation, so a struct with
    many bools will only use one or two bytes, instead of a byte per bool as is.
    
    This requires using getters and setters for the boolean fields, which changes
    user experience and is hard to document, which is a significant downside, but
    is such a nice change and will just become more and more efficient as time goes
    on.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    2365956 View commit details
    Browse the repository at this point in the history
  15. Use nonmax::NonMax* types in Options wherever possible (serenity-…

    …rs#2681)
    
    This swaps fields that store `Option<Int>` for `Option<NonMaxInt>` where the
    maximum value would be ludicrous. Since `nonmax` uses `NonZero` internally,
    this gives us niche optimisations, so model sizes can drop some more.
    
    I have had to include a workaround for [serenity-rs#17] in `optional_string` by making my
    own `TryFrom<u64>`, so that should be removable once that issue is fixed.
    
    [serenity-rs#17]: LPGhatguy/nonmax#17
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    7d641d7 View commit details
    Browse the repository at this point in the history
  16. Remove unused warning #[allow(...)]s (serenity-rs#2682)

    A couple of clippy bugs have been fixed and I have shrunk model
    sizes enough to make `clippy::large_enum_variant` go away.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    eda566c View commit details
    Browse the repository at this point in the history
  17. Get rid of unsafe (serenity-rs#2686)

    A discord bot library should not be using the tools reserved for low
    level OS interaction/data structure libraries.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    4cb59e8 View commit details
    Browse the repository at this point in the history
  18. Configuration menu
    Copy the full SHA
    d2cce74 View commit details
    Browse the repository at this point in the history
  19. Swap Id from NonZero to NonMax (serenity-rs#2689)

    Discord seems to internally default Ids to 0, which is a bug whenever
    exposed, but this makes ID parsing more resilient. I also took the
    liberty to remove the `From<NonZero*>` implementations, to prevent future
    headaches, as it was impossible to not break public API as we exposed
    `NonZero` in `*Id::parse`.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    d005efc View commit details
    Browse the repository at this point in the history
  20. Configuration menu
    Copy the full SHA
    adb9042 View commit details
    Browse the repository at this point in the history
  21. Store Request::params as a slice instead of owning ArrayVec (sere…

    …nity-rs#2694)
    
    This, 
    1. shrinks the size of Request, when copied around, as it doesn't have
    to store the max capacity at all times
    2. shrinks llvm-lines (compile time metric) for my bot in debug from
    `1,153,519` to `1,131,480` as no monomorphisation has to be performed
    for `MAX_PARAMS`.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    4273f1b View commit details
    Browse the repository at this point in the history
  22. Revert Request::params back to Option (serenity-rs#2695)

    Follow-up to serenity-rs#2694.
    
    When `Request::params` was made into an ArrayVec, the `Option` around it
    was removed in order to avoid having to add a turbofish on `None` to
    specify the value of `MAX_PARAMS`. Also, `Request::new` also needed to
    be changed so that the value of `MAX_PARAMS` could be inferred. Now that
    the field is a slice again, we can wrap it in `Option` again (at no cost
    to size, thanks to niche opts).
    
    We ensure we never store the redundant `Some(&[])` by checking for an
    empty slice and storing `None` instead. This way, we ensure we never
    read an empty slice out of the `Some` variant.
    mkrasnitski authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    a705093 View commit details
    Browse the repository at this point in the history
  23. Configuration menu
    Copy the full SHA
    a972add View commit details
    Browse the repository at this point in the history
  24. Feature gate #[instrument] macros (serenity-rs#2707)

    The instrument macros generate 2% of Serenity's release mode llvm-lines,
    and are proc-macros so hurt compile time in that way, so this limits
    them to opt-in. This commit also fixes the issues that the instrument macro
    was hiding, such as results that didn't ever error and missing
    documentation.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    32c3216 View commit details
    Browse the repository at this point in the history
  25. Remove impl Into<Option> (serenity-rs#2701)

    This signature is hard to use as `None` cannot infer the type of the
    generic. I also replaced `Option<u8>` with `Option<NonMaxU8>` as it's
    more efficient and will make the user think of the maximum value.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    93caec3 View commit details
    Browse the repository at this point in the history
  26. Get rid of IntoIterator generics wherever they are inefficent (sere…

    …nity-rs#2698)
    
    This removes inefficient `IntoIterator` generics and instead takes what is
    actually required. I also reworked `reorder_channels` to allow for keeping the
    generic, as it actually does only just need iterator semantics.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    07d2b22 View commit details
    Browse the repository at this point in the history
  27. Clean up Ratelimiter (serenity-rs#2687)

    Previously, someone assumed that `Ratelimiter` was going to be cloned, so
    put a ton of `Arc`s everywhere. This was unneeded, and before dashmap,
    so the buckets were also stored massively inefficiently. This fixes all
    that.
    
    I had to shuffle around the `Ratelimit` methods a little bit to return
    their sleep time instead of sleeping themselves, so I didn't have to
    hold a dashmap lock over an `.await`.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    09ad9f4 View commit details
    Browse the repository at this point in the history
  28. Handle overflow checks in central locations (serenity-rs#2697)

    This removes multiple error variants and overall cleans up the codebase
    by moving overflow checks into two `ModelError` variants.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    22e4c6a View commit details
    Browse the repository at this point in the history
  29. Shrink size and clean up Error (serenity-rs#2700)

    Shrinks `size_of::<Error>` from 136 bytes to 64 bytes, while removing unused
    variants. This will improve performance for any method returning
    `Result<T>` where `T` is less than the size of `Error` as both `Result`'s
    `Ok` and `Err` have to be allocated stack space.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    6f48bd0 View commit details
    Browse the repository at this point in the history
  30. Remove manual #[inline] attributes (serenity-rs#2702)

    The compiler knows best as inlining is quite complicated. This should
    help with compile times, significantly.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    c4e69e2 View commit details
    Browse the repository at this point in the history
  31. Remove Into<*Id> and AsRef<str> (serenity-rs#2704)

    `Into<*Id>` is often entirely unnecessary, and at worst hides the type of
    the Id with hard-coded Ids. This fixes this by just using the Id type
    itself, which also helps with monomorphisation bloat.
    
    I also removed `AsRef<str>`, as this can just be `&str` without
    requiring a generic and without requiring a call to as_ref in the
    function, while also not accidentally taking ownership of a `String`
    passed in misleadingly.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    6571ed4 View commit details
    Browse the repository at this point in the history
  32. Remove Event::Unknown variant (serenity-rs#2708)

    If deserialization fails, we should allow the specific deserialization error to
    be logged so that it can be reported to the Serenity maintainers. As it stands,
    any events that fail to deserialize normally will "succeed" when deserialized
    into the `Unknown` variant, throwing away any extra information about the
    error.
    
    Because deserialization errors are immediately logged when they happen,
    explicitly whitelist `Error::Json` in `Shard::handle_event` to prevent
    duplicate logs.
    mkrasnitski authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    fc3b3f3 View commit details
    Browse the repository at this point in the history
  33. Configuration menu
    Copy the full SHA
    80435a5 View commit details
    Browse the repository at this point in the history
  34. Stop using Value in error parsing (serenity-rs#2710)

    `Value` is bad to use and is slow. I also used `Arc<str>` to store `path` as
    this is cloned a couple times.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    9adbfa6 View commit details
    Browse the repository at this point in the history
  35. Fix new clippy lints (serenity-rs#2713)

    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    a3fa2a0 View commit details
    Browse the repository at this point in the history
  36. Update to small-fixed-array v0.2 (serenity-rs#2711)

    `small-fixed-array` v0.2 removes implicit truncation and makes all
    truncations explicit.
    
    I also changed FixedString initialization from literals to use
    `from_str_trunc` to take advantage of SSO if possible.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    41c4c8b View commit details
    Browse the repository at this point in the history
  37. Fix @everyone role deserialisation (serenity-rs#2716)

    The `@everyone` role sometimes has a role position of -1, but not always!
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    af0b5d5 View commit details
    Browse the repository at this point in the history
  38. Fix GuildChannel deserialisation with user_limit (serenity-rs#2715)

    This is documented as max 99 for voice channels, but 10k for stage
    channels.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    5c8ea15 View commit details
    Browse the repository at this point in the history
  39. Configuration menu
    Copy the full SHA
    48131cb View commit details
    Browse the repository at this point in the history
  40. Remove dashboard example (serenity-rs#2714)

    This example depended on rillrate, which has been archived for over a
    year.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    b444228 View commit details
    Browse the repository at this point in the history
  41. Fix compilation from rebase

    arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    04cb590 View commit details
    Browse the repository at this point in the history
  42. Configuration menu
    Copy the full SHA
    077dec3 View commit details
    Browse the repository at this point in the history
  43. Implement max_concurrency support when starting shards (serenity-rs…

    …#2661)
    
    When a session is first kicked off and we need to start many shards, we
    can use the value of `max_concurrency` to kick off multiple shards in
    parallel.
    mkrasnitski authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    225dfca View commit details
    Browse the repository at this point in the history
  44. Remove typemap without making everything generic (serenity-rs#2720)

    Replaces the typemap with `dyn Any`, which is what typemap used
    internally anyway. This gives the user more control over locking,
    removes boilerplate, and is less confusing for beginners to learn.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    d32d74d View commit details
    Browse the repository at this point in the history
  45. Provide Arc<Data> instead of &Data (serenity-rs#2722)

    This incurs an extra ref count, but this should be negligible for the
    more permissive type. An explanation of how this works is that:
    
    - `Arc` is just a shared pointer to `ArcInner`
    - `ArcInner` stores the concrete Data type
    - Each `Arc` stores the dyn vtable ptr
    - `Arc` checks the vtable ptr to get the type id, then it can just give
    out a new `Arc` referring to the `ArcInner` concretely.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    ab6d300 View commit details
    Browse the repository at this point in the history
  46. Add Client::try_data to fallibly fetch the data type (serenity-rs#2723

    )
    
    This is mostly for usage in `poise` to provide a nicer error message in
    its situation. I also changed `Client::data` to provide `Arc` as well.
    GnomedDev authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    9cc5807 View commit details
    Browse the repository at this point in the history
  47. Configuration menu
    Copy the full SHA
    0c4f5e9 View commit details
    Browse the repository at this point in the history
  48. Remove some pedantic lints from the whitelist (serenity-rs#2728)

    This also removes the impls of `From<{i32,u64}> for Colour`, as it only
    makes sense to convert infallibly from `u32`.
    mkrasnitski authored and arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    6e0a188 View commit details
    Browse the repository at this point in the history
  49. Configuration menu
    Copy the full SHA
    eef7c95 View commit details
    Browse the repository at this point in the history
  50. Configuration menu
    Copy the full SHA
    fc2f751 View commit details
    Browse the repository at this point in the history
  51. Fix compilation from rebase

    arqunis committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    679569b View commit details
    Browse the repository at this point in the history
  52. Fix testing example

    mkrasnitski committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    4f5f00b View commit details
    Browse the repository at this point in the history
  53. Remove PartialEq implementations from all builders (serenity-rs#2734)

    Because builder fields are all private, it doesn't make sense to compare
    them directly. From a user's perspective, they have no idea what the
    builder might contain. This also lets us change their internals easier,
    without worrying about equality.
    mkrasnitski committed Jan 22, 2024
    Configuration menu
    Copy the full SHA
    1f20332 View commit details
    Browse the repository at this point in the history

Commits on Jan 24, 2024

  1. Remove various instances of impl AsRef<T> (serenity-rs#2736)

    This should improve compile times even more, by compiling a lot more
    methods in Serenity, instead of the user's code.
    GnomedDev committed Jan 24, 2024
    Configuration menu
    Copy the full SHA
    e9b699c View commit details
    Browse the repository at this point in the history
  2. Remove simd-json support (serenity-rs#2735)

    `simd-json` is not a performance improvement for Serenity.
    
    It has a ton of sketchy uses of `unsafe`, increases the maintenance burden, and
    leads to Serenity containing a json compatibility layer(!). 
    
    This commit removes all that, although a faster json decoding implementation is
    on the table in the future if the compatibility layer is kept out of tree and
    it is measurably faster for Serenity's usecase.
    GnomedDev committed Jan 24, 2024
    Configuration menu
    Copy the full SHA
    4e05d38 View commit details
    Browse the repository at this point in the history

Commits on Jan 25, 2024

  1. Fix massive code size of client::dispatch (serenity-rs#2739)

    This replaces fancy iter stuff with just writing the code, and therefore
    reduces the code size significantly and spawns less tasks.
    GnomedDev committed Jan 25, 2024
    Configuration menu
    Copy the full SHA
    6ba5170 View commit details
    Browse the repository at this point in the history

Commits on Jan 26, 2024

  1. Fix RoleTags deserialisation after removal of simd-json (serenity-rs#…

    …2742)
    
    There was a comment here that said it was called by simd-json, so I
    removed the function, but it turns out that serde_json also calls it.
    GnomedDev committed Jan 26, 2024
    Configuration menu
    Copy the full SHA
    a4a06a4 View commit details
    Browse the repository at this point in the history

Commits on Jan 27, 2024

  1. Avoid cloning FullEvent in dispatch code (serenity-rs#2740)

    Avoids cloning entire event structs when dispatching. To reduce code
    size and allocations all the state is bundled up into a temporary struct
    called DispatchContext, but this isn't user facing at all. This
    regresses code size a bit, but is worth it for runtime performance, and
    probably reduces actual code sizes as `FullEvent` doesn't have to be
    moved around as much.
    GnomedDev committed Jan 27, 2024
    Configuration menu
    Copy the full SHA
    1b78c2c View commit details
    Browse the repository at this point in the history
  2. Remove the builder trait and only take required arguments (serenity-r…

    …s#2741)
    
    Removes unnecessary trait, which only limited the signatures to always
    take `CacheHttp` and required the async_trait boxing and compile time
    cost. This also makes a lot less methods generic, improving compile
    times.
    GnomedDev committed Jan 27, 2024
    Configuration menu
    Copy the full SHA
    e79e5d0 View commit details
    Browse the repository at this point in the history

Commits on Jan 28, 2024

  1. Configuration menu
    Copy the full SHA
    419be9a View commit details
    Browse the repository at this point in the history

Commits on Jan 29, 2024

  1. Convert enum number into wrapper struct to save type sizes (serenity-…

    …rs#2746)
    
    Saves a couple bytes in each model by replacing the enums with a simpler
    and more accurate representation, a newtype with named statics.
    https://www.diffchecker.com/ohzayL9Z/
    GnomedDev committed Jan 29, 2024
    Configuration menu
    Copy the full SHA
    e1f90df View commit details
    Browse the repository at this point in the history
  2. Use Arc<str> to store the token (serenity-rs#2745)

    `SecretString` is nice to have, but this is more efficient, and
    `SecretString` is defense in depth we can do without.
    GnomedDev committed Jan 29, 2024
    Configuration menu
    Copy the full SHA
    a7e405e View commit details
    Browse the repository at this point in the history
  3. Remove standard framework (serenity-rs#2731)

    This PR carries out the removal after the deprecation made on current.
    GnomedDev committed Jan 29, 2024
    Configuration menu
    Copy the full SHA
    d706897 View commit details
    Browse the repository at this point in the history
  4. Re-add Secret to bot token (serenity-rs#2748)

    This re-adds Secret to the bot token, without losing the `Arc<str>` opt.
    This works by using a newtype to Zeroise on last drop, using
    `Arc::get_mut`.
    GnomedDev committed Jan 29, 2024
    Configuration menu
    Copy the full SHA
    f8fed24 View commit details
    Browse the repository at this point in the history

Commits on Feb 2, 2024

  1. Configuration menu
    Copy the full SHA
    2ad6c78 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    058d7e5 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    8aad616 View commit details
    Browse the repository at this point in the history