Impl get_size::GetSize (behind feature flag) (v1 branch) by amandasaurus · Pull Request #335 · servo/rust-smallvec

amandasaurus · 2024-01-17T16:35:52Z

Shows memory usage of the struct

cf. issue #331

This includes unittests, and I have briefly used this in real programmes successfully.

mbrubeck · 2024-01-17T17:15:56Z

src/tests.rs

    assert_eq!(a, SmallVec::<[i32; 3]>::from_slice(&[1i32, 3, 5, 6, 7, 8]));
 }
+
+#[cfg(feature = "get-size")]


It looks like this test is failing when both the "get-size" and "union" features are enabled. You can reproduce this failure by running cargo test --features get-size,union.

The test will also fail on non-64-bit platforms. You can fix both issues by changing this line to something like

#[cfg(all(feature = "get-size", target_pointer_with = "64", not(feature = "union")))]

👍 Fixed. & new branch (w. clean history) pushed. I've also added some tests for when the union feature is active.

Shows memory usage of the struct cf. issue servo#331

mbrubeck · 2024-01-18T20:07:46Z

Thanks! Published as smallvec 1.13.0.

amandasaurus · 2024-01-19T13:13:50Z

Unfortunately, this has a bug.

a Cargo.toml with:

[dependencies]
get-size = { version = "0.1.4", features = ["derive"] }
smallvec = { version = "1.13.0", features = ["get-size"] }

will fail to compile with:

error: cyclic package dependency: package `attribute-derive v0.6.1` depends on itself. Cycle:
package `attribute-derive v0.6.1`
    ... which satisfies dependency `attribute-derive = "^0.6"` (locked to 0.6.1) of package `get-size-derive v0.1.3`
    ... which satisfies dependency `get-size-derive = "^0.1.3"` (locked to 0.1.3) of package `get-size v0.1.4`
    ... which satisfies dependency `get-size = "^0.1"` (locked to 0.1.4) of package `smallvec v1.13.0`
    ... which satisfies dependency `smallvec = "^1.5"` (locked to 1.13.0) of package `proc-macro-utils v0.8.0`
    ... which satisfies dependency `proc-macro-utils = "^0.8.0"` (locked to 0.8.0) of package `attribute-derive-macro v0.6.1`
    ... which satisfies dependency `attribute-derive-macro = "^0.6.1"` (locked to 0.6.1) of package `attribute-derive v0.6.1`

However these dependencies works:

[dependencies]
get-size = { version = "0.1.4", features = ["derive"] }
smallvec = { git = "https://github.com/servo/rust-smallvec.git", rev="v1.13.0", features = ["get-size"] }

😖😖 But the v1.13.0 tag should be the same as the released 1.13.0, right?

mbrubeck · 2024-01-19T15:49:38Z

Hmm, that's annoying. As a workaround you can do this:

[dependencies]
get-size = "0.1.4"
get-size-derive = "0.1"
smallvec = { version = "1.13.0", features = ["get-size"] }

and replace #[derive(GetSize)] in your code with:

#[derive(get_size_derive::GetSize)]

but things will still break if you have dependencies outside of your control that enable the "derive" feature of get-size.

Perhaps it would be better to remove these impls from the smallvec crate, and add them to the get-size crate, if the author is willing.

But the v1.13.0 tag should be the same as the released 1.13.0, right?

Using a git dependency breaks the cycle because Cargo considers it a different crate than the crates.io dependency, so it ends up building two different copies of the smallvec code.

mbrubeck · 2024-01-19T16:39:12Z

I have reverted this change and yanked version 1.13.0 for now, until we figure out the best solution. Sorry about this. :(

mbrubeck reviewed Jan 17, 2024

View reviewed changes

mbrubeck mentioned this pull request Jan 17, 2024

Impl get_size::GetSize (behind feature flag) (v2 branch) #336

Merged

Impl get_size::GetSize (behind feature flag)

b6b55c0

Shows memory usage of the struct cf. issue servo#331

amandasaurus force-pushed the get-size-v1 branch from 40460fc to b6b55c0 Compare January 18, 2024 10:26

mbrubeck approved these changes Jan 18, 2024

View reviewed changes

mbrubeck merged commit 38863e8 into servo:v1 Jan 18, 2024

mbrubeck mentioned this pull request Jan 18, 2024

implement get_size::GetSize for SmallVec, so we can know how much memory we're using? #331

Closed

amandasaurus mentioned this pull request Jan 22, 2024

impl GetSize for external crate, behind a feature flag in this crate? DKerp/get-size#16

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Impl get_size::GetSize (behind feature flag) (v1 branch)#335

Impl get_size::GetSize (behind feature flag) (v1 branch)#335
mbrubeck merged 1 commit intoservo:v1from
amandasaurus:get-size-v1

amandasaurus commented Jan 17, 2024

Uh oh!

mbrubeck Jan 17, 2024

Uh oh!

amandasaurus Jan 18, 2024

Uh oh!

mbrubeck commented Jan 18, 2024

Uh oh!

amandasaurus commented Jan 19, 2024

Uh oh!

mbrubeck commented Jan 19, 2024

Uh oh!

mbrubeck commented Jan 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

amandasaurus commented Jan 17, 2024

Uh oh!

mbrubeck Jan 17, 2024

Choose a reason for hiding this comment

Uh oh!

amandasaurus Jan 18, 2024

Choose a reason for hiding this comment

Uh oh!

mbrubeck commented Jan 18, 2024

Uh oh!

amandasaurus commented Jan 19, 2024

Uh oh!

mbrubeck commented Jan 19, 2024

Uh oh!

mbrubeck commented Jan 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants