Consider making more dependencies opt-in

[matklad]

At the moment, insta library pulls quite a few deps. Specifically, in my Cargo.lock I see

dependencies = [
 "chrono 0.4.7 (registry+https://github.com/rust-lang/crates.io-index)",
 "ci_info 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)",
 "console 0.7.7 (registry+https://github.com/rust-lang/crates.io-index)",
 "difference 2.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
 "failure 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
 "lazy_static 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
 "pest 2.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
 "pest_derive 2.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
 "ron 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)",
 "serde 1.0.99 (registry+https://github.com/rust-lang/crates.io-index)",
 "serde_json 1.0.40 (registry+https://github.com/rust-lang/crates.io-index)",
 "serde_yaml 0.8.9 (registry+https://github.com/rust-lang/crates.io-index)",
 "uuid 0.7.4 (registry+https://github.com/rust-lang/crates.io-index)",
]

insta is dev-dependency, so those transtive deps don't leak into the final executable, but they still affect compile time and the size of the ./target dir, making overall CI time larger.

I think I understand why each of those dependencies is there, but, at the same time, some of them look like that could have been avoided or made optional.

Specifically:

• json, yaml and ron are kind of the same thing, perhaps serialziation formats can be opt-in?
• pestis used for redactions, but it's a heavy dependency, could redactions be made a feature?
• failure slightly out of place in the library, perhaps sticking with std::error will work?
• ci_info looks pretty entreprisy, Cargo just checks two env vars: https://github.com/rust-lang/cargo/blob/1dd02155c988707b3e12606f52b29a90c76520dd/src/cargo/util/mod.rs#L86-L89
• chrono is also heavy, and, I think, it is only used to format time in snap files. In this case, I don't know how to improve this, except for removing the time field altogether.

[max-sixty]

These are the stats by dependency:

cargo bloat --time                                                                                                                                      Thu Aug 22 08:17:59 2019
Compiling ...

  Time Crate
14.81s syn
14.65s regex_syntax
13.22s syn
10.59s serde
 9.49s serde_derive
 8.03s regex
 7.81s pest_meta
 6.69s aho_corasick
 6.25s cc
 5.34s proc_macro2
 5.33s proc_macro2
 5.29s yaml_rust
 4.66s rand
 4.22s pest
 3.46s num_traits
 3.35s insta
 3.35s serde_json
 3.31s pest_generator
 3.08s synstructure
 3.04s failure_derive
 2.80s memchr
 2.70s chrono
 2.56s byteorder
 2.53s libc
 2.43s serde_yaml
 2.39s console
 2.30s ron
 2.25s parking_lot
 1.96s parking_lot_core
 1.85s time
 1.82s num_integer
 1.71s backtrace
 1.67s rustc_demangle
 1.67s semver
 1.56s semver_parser
 1.53s ryu
 1.49s quote
 1.43s autocfg
 1.43s ucd_trie
 1.39s pest_derive
 1.13s rustc_version
 1.12s backtrace_sys
 1.10s difference
 1.02s quote
 1.01s base64
 1.01s rand_chacha
 1.00s uuid
 0.96s bitflags
 0.89s smallvec
 0.83s rand_hc
 0.83s rand_isaac
 0.82s thread_local
 0.79s rand_pcg
 0.78s rand_core
 0.68s lock_api
 0.64s linked_hash_map
 0.59s rand_jitter
 0.54s clicolors_control
 0.44s dtoa
 0.42s rand_os
 0.41s failure
 0.40s termios
 0.37s itoa
 0.36s rand_xorshift
 0.36s ci_info
 0.21s unicode_xid
 0.21s unicode_xid
 0.19s lazy_static
 0.18s unicode_width
 0.15s atty
 0.14s scopeguard
 0.11s maplit
 0.09s rand_core
 0.08s cfg_if

It looks like the big difference would be syn, which is used for parsing inline snapshots. Could we make inline snapshots a default optional feature?

Redactions might be easier to make optional, but it doesn't look like it would have a large impact. yaml / json probably both harder to make optional and more marginal.

Any of the others look like there's potential @matklad ?

[matklad]

but it doesn't look like it would have a large impact.

Hm, If I sum the three pest entries, I get about 13 seconds. And I think time to compile pest-generated grammar itself might not be included in the picture above?

It looks like the big difference would be syn, which is used for parsing inline snapshots.

It is used by cargo-insta binary, and dependencies there are much less important, b/c one does not build cargo-insta on CI. That's why I've specifically listed the deps of the library.

[max-sixty]

Hm, If I sum the three pest entries, I get about 13 seconds. And I think time to compile pest-generated grammar itself might not be included in the picture above?

Right right, I agree that's something!

It is used by cargo-insta binary, and dependencies there are much less important, b/c one does not build cargo-insta on CI. That's why I've specifically listed the deps of the library.

Ah interesting. Do you know whether there's a way of measuring the insta impact alone?
Out of interest do you know why syn appears in the list on running cargo build? Are all workspaces` dependencies built?

[max-sixty]

ref https://raphlinus.github.io/rust/2019/08/21/rust-bloat.html

[matklad]

Out of interest do you know why syn appears in the list on running cargo build?

from failure_derive at least

[max-sixty]

from failure_derive at least

Thanks. Is there a way of confirming whether that's the only case? (Or can you at least estimate that?). I could do a PR to change Fail to Error if that's the only cause of syn, and @mitsuhiko thinks it's worthwhile

[mitsuhiko]

More than happy to drop failure.

[mitsuhiko]

So getting rid of syn will be hard. It's used for the internal use of serde derive and doing this manually really sucks. Since almost everyone uses serde anyways i'm not sure if there is a lot of value trying to remove this.

[matklad]

Agree that getting rid of serde it's not very important, that's why I didn't list it as a suspicious dependency.

I am more concerned about serde_yaml, which I think is pretty heavy (as yaml is big), and is unlikely to be used by anything else. But I see that yaml is pretty important for on-disk snapshots. Don't have any bright ideas here except for:

• make on-disk snapshots a cargo-feature (which obviously wouldn't work if you want the snapshots)
• replace yaml with something add-hock like (header: value\n)*----\n. That would be slightly painful to do, but shouldn't be too bad, given the fact that, unlike the general serilization, both producer and consumer are the same.

[mitsuhiko]

So I made RON optional now which was easy. Originally yaml was not used for the on disk serialization but that made it really hard to consume the snapshots from other tools and yaml is too hard to implement yourself. Not sure what a good way here is.

[matklad]

Yeah, interop with other tools is a good reason for yaml, haven't realized that that's important.

The subset of yaml insta actually uses still looks relatively simple and hand-rollable, but not trivially hand-rollable:

---
created: "2019-07-20T20:13:53.385368Z"
creator: insta@0.8.1
source: crates/ra_ide_api/src/inlay_hints.rs
expression: hints
---

[mitsuhiko]

FWIW serde_yaml is pretty light all things considered. It has very few dependencies.

[matklad]

I stand corrected, thanks! According to tokei and quick look with a naked eye, serde_yaml (3k sloc) is indeed more lean than sere_json (9k sloc).

[mitsuhiko]

This is what it looks like now. I don't think it can be cut down further without removing chrono which is tricky:

testing v0.1.0 (/private/tmp/testing)
└── insta v0.10.1 (/Users/mitsuhiko/Development/insta)
    ├── chrono v0.4.7
    │   ├── libc v0.2.62
    │   ├── num-integer v0.1.41
    │   │   └── num-traits v0.2.8
    │   │       [build-dependencies]
    │   │       └── autocfg v0.1.6
    │   │   [build-dependencies]
    │   │   └── autocfg v0.1.6 (*)
    │   ├── num-traits v0.2.8 (*)
    │   ├── serde v1.0.99
    │   │   └── serde_derive v1.0.99
    │   │       ├── proc-macro2 v1.0.1
    │   │       │   └── unicode-xid v0.2.0
    │   │       ├── quote v1.0.2
    │   │       │   └── proc-macro2 v1.0.1 (*)
    │   │       └── syn v1.0.3
    │   │           ├── proc-macro2 v1.0.1 (*)
    │   │           ├── quote v1.0.2 (*)
    │   │           └── unicode-xid v0.2.0 (*)
    │   │   [dev-dependencies]
    │   │   └── serde_derive v1.0.99 (*)
    │   └── time v0.1.42
    │       └── libc v0.2.62 (*)
    │       [dev-dependencies]
    │       └── winapi v0.3.7
    ├── console v0.8.0
    │   ├── clicolors-control v1.0.1
    │   │   ├── lazy_static v1.3.0
    │   │   └── libc v0.2.62 (*)
    │   ├── lazy_static v1.3.0 (*)
    │   ├── libc v0.2.62 (*)
    │   ├── regex v1.2.1
    │   │   ├── aho-corasick v0.7.6
    │   │   │   └── memchr v2.2.1
    │   │   ├── memchr v2.2.1 (*)
    │   │   ├── regex-syntax v0.6.11
    │   │   └── thread_local v0.3.6
    │   │       └── lazy_static v1.3.0 (*)
    │   ├── termios v0.3.1
    │   │   └── libc v0.2.62 (*)
    │   └── unicode-width v0.1.6
    ├── difference v2.0.0
    ├── lazy_static v1.3.0 (*)
    ├── serde v1.0.99 (*)
    ├── serde_json v1.0.40
    │   ├── itoa v0.4.4
    │   ├── ryu v1.0.0
    │   └── serde v1.0.99 (*)
    ├── serde_yaml v0.8.9
    │   ├── dtoa v0.4.4
    │   ├── linked-hash-map v0.5.2
    │   ├── serde v1.0.99 (*)
    │   └── yaml-rust v0.4.3
    │       └── linked-hash-map v0.5.2 (*)
    └── uuid v0.7.4
        ├── rand v0.6.5
        │   ├── libc v0.2.62 (*)
        │   ├── rand_chacha v0.1.1
        │   │   └── rand_core v0.3.1
        │   │       └── rand_core v0.4.2
        │   │   [build-dependencies]
        │   │   └── autocfg v0.1.6 (*)
        │   ├── rand_core v0.4.2 (*)
        │   ├── rand_hc v0.1.0
        │   │   └── rand_core v0.3.1 (*)
        │   ├── rand_isaac v0.1.1
        │   │   └── rand_core v0.3.1 (*)
        │   ├── rand_jitter v0.1.4
        │   │   ├── libc v0.2.62 (*)
        │   │   └── rand_core v0.4.2 (*)
        │   ├── rand_os v0.1.3
        │   │   ├── libc v0.2.62 (*)
        │   │   └── rand_core v0.4.2 (*)
        │   ├── rand_pcg v0.1.2
        │   │   └── rand_core v0.4.2 (*)
        │   │   [build-dependencies]
        │   │   └── autocfg v0.1.6 (*)
        │   └── rand_xorshift v0.1.1
        │       └── rand_core v0.3.1 (*)
        │   [build-dependencies]
        │   └── autocfg v0.1.6 (*)
        └── serde v1.0.99 (*)

[matklad]

Nice, thanks! Note that I've already send a PR to uuid to bump rand to 0.7, which should further reduce deps and remove duplication with other modern rand users. The PR was merged, but I don't know when the next release of uuid is scheduled.

[kinggoesgaming]

Nice, thanks! Note that I've already send a PR to uuid to bump rand to 0.7, which should further reduce deps and remove duplication with other modern rand users. The PR was merged, but I don't know when the next release of uuid is scheduled.

I am trying to find time to release 0.8 version as soon as I can. There is one more refactor that needs to be done for the release and then its all good to go.