feat(info): Dependency count and sizes #6786

KrisChambers · 2020-07-17T21:13:33Z

This pr addresses issue #6468. Adding counts and size reporting for the modules.

Just looking for some feedback on a few things.

I have noticed the following:

deno info currently doesn't include any files with specifiers starting with "file://". Is this a bug or as intended?
The output format of the current json felt a bit weird with adding the extra informaton so here is a possible alternative schema with the extra information included.
The output of the pr follows the format suggested in Feature request: Add size and count of unique deps to Deno info #6468 by @cknight. Generally this is great, but I think there could be a source of confusion about what is meant when we say total.

For 1:
The implementation in the pr uses ModuleGraph instead of Deps which contains modules with "file://" as their specifier. This is different from the output I was getting on the live version of deno info command.

For 2:
We are generating a tree from ModuleGraph which derives serde::Serialize. This gives a json formatting for free.

Here is some example output:

{
  "local": "/home/k/Code/deno/my_test/server.ts",
  "fileType": "TypeScript",
  "compiled": "/home/k/.cache/deno/gen/file/home/k/Code/deno/my_test/server.ts.js",
  "map": null,
  "deps": {
    "name": "file:///home/k/Code/deno/my_test/server.ts",
    "size": 127,
    "total_size": 63462,
    "deps": [
      {
        "name": "https://deno.land/std/http/server.ts",
        "size": 10438,
        "total_size": 63123,
        "deps": [
          {
            "name": "https://deno.land/std/encoding/utf8.ts",
            "size": 433,
            "total_size": 433,
            "deps": []
          },
          {
            "name": "https://deno.land/std/io/bufio.ts",
            "size": 21652,
            "total_size": 26505,
            "deps": [
              {
                "name": "https://deno.land/std/bytes/mod.ts",
                "size": 4448,
                "total_size": 4448,
                "deps": []
              },
              {
                "name": "https://deno.land/std/_util/assert.ts",
                "size": 405,
                "total_size": 405,
                "deps": []
              }
            ]
          },
          {
            "name": "https://deno.land/std/_util/assert.ts",
            "size": 405,
            "total_size": 0,
            "deps": []
          },
...

It was brought up in the pr #6372 to make the flag unstable due to possible iterations on the schema. Any suggestions would be great. Personally, I think the above works a bit better and is more easily extensible. I am also assuming that the intention of the json flag is to have it's output piped into something else, so maybe this would also be easier to consume.

For 3:
The current state of the pr outputs something like this:

file:///home/k/Code/deno/my_test/server.ts (127 Bytes, total = 61.97 KB)
├─┬ https://deno.land/std/http/server.ts (10.19 KB, total = 61.64 KB)
│ ├── https://deno.land/std/encoding/utf8.ts (433 Bytes, total = 433 Bytes)
│ ├─┬ https://deno.land/std/io/bufio.ts (21.14 KB, total = 25.88 KB)
│ │ ├── https://deno.land/std/bytes/mod.ts (4.34 KB, total = 4.34 KB)
│ │ └── https://deno.land/std/_util/assert.ts (405 Bytes, total = 405 Bytes)
│ ├── https://deno.land/std/_util/assert.ts (405 Bytes, total = 0 Bytes)
│ ├─┬ https://deno.land/std/async/mod.ts (175 Bytes, total = 3.46 KB)
│ │ ├── https://deno.land/std/async/deferred.ts (1.03 KB, total = 1.03 KB)
│ │ ├── https://deno.land/std/async/delay.ts (279 Bytes, total = 279 Bytes)
│ │ └─┬ https://deno.land/std/async/mux_async_iterator.ts (1.98 KB, total = 1.98 KB)
│ │   └── https://deno.land/std/async/deferred.ts (1.03 KB, total = 0 Bytes)
│ └─┬ https://deno.land/std/http/_io.ts (11.24 KB, total = 21.68 KB)
│   ├── https://deno.land/std/io/bufio.ts (21.14 KB, total = 0 Bytes)
│   ├─┬ https://deno.land/std/textproto/mod.ts (4.51 KB, total = 4.51 KB)
│   │ ├── https://deno.land/std/io/bufio.ts (21.14 KB, total = 0 Bytes)
│   │ ├── https://deno.land/std/bytes/mod.ts (4.34 KB, total = 0 Bytes)
│   │ └── https://deno.land/std/encoding/utf8.ts (433 Bytes, total = 0 Bytes)
│   ├── https://deno.land/std/_util/assert.ts (405 Bytes, total = 0 Bytes)
│   ├── https://deno.land/std/encoding/utf8.ts (433 Bytes, total = 0 Bytes)
│   ├── https://deno.land/std/http/server.ts (10.19 KB, total = 0 Bytes)
│   └── https://deno.land/std/http/http_status.ts (5.93 KB, total = 5.93 KB)
└─┬ file:///home/k/Code/deno/my_test/a.ts (52 Bytes, total = 212 Bytes)
  └─┬ file:///home/k/Code/deno/my_test/b.ts (52 Bytes, total = 160 Bytes)
    └─┬ file:///home/k/Code/deno/my_test/c.ts (52 Bytes, total = 108 Bytes)
      └─┬ file:///home/k/Code/deno/my_test/d.ts (56 Bytes, total = 56 Bytes)
        └── file:///home/k/Code/deno/my_test/a.ts (52 Bytes, total = 0 Bytes)

Currently, if we already know the size of something, I am just returning a 0 total size. As in, "this reference is contributing 0 to the total size". I don't know if this is all that intuitive to the end user just by looking at this output or if this is the intended use of "total".

Setting things to 0 is mainly being used to get around circular references.

So here are some options.

Use the above formatting.
Report the sizes once on the tree: This might give a clearer indication that we are calculating the total size of the module based on it's dependencies.

The output would look something like this:

file:///home/k/Code/deno/my_test/server.ts (127 Bytes, total = 61.97 KB)
├─┬ https://deno.land/std/http/server.ts (10.19 KB, total = 61.64 KB)
│ ├── https://deno.land/std/encoding/utf8.ts (433 Bytes, total = 433 Bytes)
│ ├─┬ https://deno.land/std/io/bufio.ts (21.14 KB, total = 25.88 KB)
│ │ ├── https://deno.land/std/bytes/mod.ts (4.34 KB, total = 4.34 KB)
│ │ └── https://deno.land/std/_util/assert.ts (405 Bytes, total = 405 Bytes)
│ ├── https://deno.land/std/_util/assert.ts
│ ├─┬ https://deno.land/std/async/mod.ts (175 Bytes, total = 3.46 KB)
│ │ ├── https://deno.land/std/async/deferred.ts (1.03 KB, total = 1.03 KB)
│ │ ├── https://deno.land/std/async/delay.ts (279 Bytes, total = 279 Bytes)
│ │ └─┬ https://deno.land/std/async/mux_async_iterator.ts (1.98 KB, total = 1.98 KB)
│ │   └── https://deno.land/std/async/deferred.ts
│ └─┬ https://deno.land/std/http/_io.ts (11.24 KB, total = 21.68 KB)
│   ├── https://deno.land/std/io/bufio.ts
│   ├─┬ https://deno.land/std/textproto/mod.ts (4.51 KB, total = 4.51 KB)
│   │ ├── https://deno.land/std/io/bufio.ts
│   │ ├── https://deno.land/std/bytes/mod.ts
│   │ └── https://deno.land/std/encoding/utf8.ts
│   ├── https://deno.land/std/_util/assert.ts
│   ├── https://deno.land/std/encoding/utf8.ts
│   ├── https://deno.land/std/http/server.ts
│   └── https://deno.land/std/http/http_status.ts (5.93 KB, total = 5.93 KB)
└─┬ file:///home/k/Code/deno/my_test/a.ts (52 Bytes, total = 212 Bytes)
  └─┬ file:///home/k/Code/deno/my_test/b.ts (52 Bytes, total = 160 Bytes)
    └─┬ file:///home/k/Code/deno/my_test/c.ts (52 Bytes, total = 108 Bytes)
      └─┬ file:///home/k/Code/deno/my_test/d.ts (56 Bytes, total = 56 Bytes)
        └── file:///home/k/Code/deno/my_test/a.ts

Print the size of each node in the tree as if deno info <module_name> was run on it: This gives you sizes of each submodule as they are individually, but no indication to how they contribute to the overall module's size.

Personally, I think I lean more to 2 since I think my intention with using something like deno info foo.ts means I want info regarding the module foo.ts. So seeing a bunch of totals not really connected to the total of the module being talked about seems like more info than is needed.

We could also report just the total on the root module and nothing on the nodes of the tree.

Any thoughts on this would be great!

cknight · 2020-07-21T21:44:16Z

Thanks for all your work @KrisChambers this is looking great. Like you I prefer your option 2, reporting on the size of the module (and the total size with it's subdependencies) once and leaving blank for further appearances in the tree. An alternative would be to add (repeat), (duplicate), (see above) or some such. E.g.

file:///home/k/Code/deno/my_test/server.ts (127 Bytes, total = 61.97 KB)
├─┬ https://deno.land/std/http/server.ts (10.19 KB, total = 61.64 KB)
│ ├── https://deno.land/std/encoding/utf8.ts (433 Bytes, total = 433 Bytes)
│ ├─┬ https://deno.land/std/io/bufio.ts (21.14 KB, total = 25.88 KB)
│ │ ├── https://deno.land/std/bytes/mod.ts (4.34 KB, total = 4.34 KB)
│ │ └── https://deno.land/std/_util/assert.ts (405 Bytes, total = 405 Bytes)
│ ├── https://deno.land/std/_util/assert.ts (see above)

However, maybe just keep it simple for now and not adorn the duplicate entries with anything as per your original option 2 suggestion. That would make for a good starting point which could be iterated on later if needed.

KrisChambers · 2020-07-22T02:20:00Z

Thanks for your feedback @cknight. I like the idea of putting some indicator on the end. I think you are right though, and for now I might just bold the ones that are contributing to the size just for a little extra pop.

Do you have any suggestion / idea regarding the JSON output?

cknight · 2020-07-22T20:07:11Z

Having carefully looked at the JSON output proposed here vs #6372 what's demonstrated above does look better to me, both in structure and extensibility. However, I'm not conversant enough in Rust to comment on the implementations. There's still the question of what to do with repeated dependencies in the JSON output, not sure what works best there, but I'm guessing leave it blank as proposed for the non-JSON output.

KrisChambers · 2020-07-24T04:18:11Z

To summarize the changes in this pull request:

Switched the implementation to be based off ModuleGraph in the deno.cli instead of Deps in deno.core. Left Deps untouched.
The standard output looks as follows:

local: /home/k/Code/deno/cli/tests/info_recursive_imports_test.ts
type: TypeScript
compiled: /home/k/.cache/deno/gen/file/home/k/Code/deno/cli/tests/info_recursive_imports_test.ts.js
deps: 4 unique
file:///home/k/Code/deno/cli/tests/info_recursive_imports_test.ts (87 Bytes, total = 481 Bytes)
  └─┬ file:///home/k/Code/deno/cli/tests/recursive_imports/A.ts (114 Bytes, total = 394 Bytes)
    ├─┬ file:///home/k/Code/deno/cli/tests/recursive_imports/B.ts (114 Bytes, total = 280 Bytes)
    │ ├─┬ file:///home/k/Code/deno/cli/tests/recursive_imports/C.ts (132 Bytes, total = 166 Bytes)
    │ │ ├── file:///home/k/Code/deno/cli/tests/recursive_imports/A.ts
    │ │ └── file:///home/k/Code/deno/cli/tests/recursive_imports/common.ts (34 Bytes, total = 34 Bytes)
    │ └── file:///home/k/Code/deno/cli/tests/recursive_imports/common.ts
    └── file:///home/k/Code/deno/cli/tests/recursive_imports/common.ts

Where lines without totals reported are duplicates that occur earlier in the tree.

To accommodate values in the json output the schema is modified to look like the following:

{
  "local": "/home/k/Code/deno/cli/tests/info_recursive_imports_test.ts",
  "fileType": "TypeScript",
  "compiled": "/home/k/.cache/deno/gen/file/home/k/Code/deno/cli/tests/info_recursive_imports_test.ts.js",
  "map": null,
  "deps": {
    "name": "file:///home/k/Code/deno/cli/tests/info_recursive_imports_test.ts",
    "size": 87,
    "total_size": 481,
    "deps": [
      {
        "name": "file:///home/k/Code/deno/cli/tests/recursive_imports/A.ts",
        "size": 114,
        "total_size": 394,
        "deps": [
          {
            "name": "file:///home/k/Code/deno/cli/tests/recursive_imports/B.ts",
            "size": 114,
            "total_size": 280,
            "deps": [
              {
                "name": "file:///home/k/Code/deno/cli/tests/recursive_imports/C.ts",
                "size": 132,
                "total_size": 166,
                "deps": [
                  {
                    "name": "file:///home/k/Code/deno/cli/tests/recursive_imports/A.ts",
                    "size": 114,
                    "total_size": 0,
                    "deps": []
                  },
                  {
                    "name": "file:///home/k/Code/deno/cli/tests/recursive_imports/common.ts",
                    "size": 34,
                    "total_size": 34,
                    "deps": []
                  }
                ]
              },
              {
                "name": "file:///home/k/Code/deno/cli/tests/recursive_imports/common.ts",
                "size": 34,
                "total_size": 0,
                "deps": []
              }
            ]
          },
          {
            "name": "file:///home/k/Code/deno/cli/tests/recursive_imports/common.ts",
            "size": 34,
            "total_size": 0,
            "deps": []
          }
        ]
      }
    ]
  }
}

SyrupThinker · 2020-07-24T14:02:12Z

There's lots of duplicated sizes here that could be moved out like this:

{
  "...": "...",
  "deps": ["self", ["module"]],
  "metadata": {
    "self": {
      "size": 128
    },
    "module": {
      "size": 1024
    }
  }
}

IMO the total_size field is not needed in the JSON output because

Its easy to calculate
There are several possible ways of calculating it depending on what the consumer wants to determine (absolute size, contributed size)

KrisChambers · 2020-07-25T22:49:44Z

I agree there is a bunch of duplicated information here. A recent push has some of it removed similar to what is going on in the standard deno info output.

I like your idea. Instead of using the tree structure we could just provide the adjacency list of the ModuleGraph. So it would end up looking something like:

{
...
"deps": {
    "self": ["mod1", "mod2"],
    "mod1": ["mod3", "mod4"],
    "mod2": ["mod4"],
    "mod3: [],
    "mod4": ["mod2"]
},

"metadata": {
   "self": {
       "size": ...
    } ,
   ...
}
}

If we wanted to go a bit further down the rabbit hole, we could use numeric ids and even put name in the metadata.

{
...
"adjlist": {
    0: [1, 2],
    1: [3, 4],
    2: [4],
    3: [],
    4: [2]
},

"metadata": {
   0: {
       "name": "self",
       "size": ...
    } ,
   1: { ... },
   ...
}
}

CLAassistant · 2020-07-27T23:49:39Z

All committers have signed the CLA.

bartlomieju · 2020-08-03T10:53:43Z

@KrisChambers sorry for slow response, I will give this PR a review in the next days to get it into v1.3.0 next week

KrisChambers · 2020-08-05T13:17:44Z

No problem, @bartlomieju. Let me know if you have any thoughts regarding the json format.

cli/main.rs

ry · 2020-08-12T15:30:52Z

@KrisChambers Very cool feature. I can't wait to have this in the release.

When I tried this patch out I got this:

> cargo run -- info https://deno.land/std/http/file_server.ts
    Finished dev [unoptimized + debuginfo] target(s) in 0.22s
     Running `target/debug/deno info 'https://deno.land/std/http/file_server.ts'`
local: /Users/rld/Library/Caches/deno/deps/https/deno.land/93365353fdab1ce0009ac74c0900e9ad4ea0676c3cbb14fcc1c3d45b40d511d7
type: TypeScript
compiled: /Users/rld/Library/Caches/deno/gen/https/deno.land/93365353fdab1ce0009ac74c0900e9ad4ea0676c3cbb14fcc1c3d45b40d511d7.js
deps: 24 unique
https://deno.land/std/http/file_server.ts (0 Bytes, total = 0 Bytes)

This seems broken... any idea what's happening?

KrisChambers · 2020-08-13T04:32:01Z

Looks like there is a small problem with the versioning. Specifying the version (https://deno.land/std@0.63.0/http/file_server.ts) looks like it works. I will try and get this fixed tomorrow.

bartlomieju · 2020-08-13T14:28:48Z

I've removed 1.3.0 milestone and slated for 1.4.0, this functionality is desired but needs more work.

bartlomieju · 2020-09-02T11:13:39Z

We might want to address #4494 in this PR as well - it's a matter of a single flag on ModuleGraphLoader

lucacasonato · 2020-09-04T13:33:21Z

bartlomieju · 2020-09-04T15:11:44Z

We might want to address #4494 in this PR as well - it's a matter of a single flag on ModuleGraphLoader

This issue has been handled in this PR as well.

Closes #4494

bartlomieju · 2020-09-04T19:53:10Z

Still need to add a test for output when compiled source code is available.

bartlomieju · 2020-09-07T12:13:29Z

This PR should be ready to land

ry · 2020-09-07T13:48:51Z

core/modules.rs

-    assert_eq!(bar_deps.deps, Some(vec![]));
-  }
-
-  */


great to see this moved out of core

ry

LGTM - thank you @KrisChambers and @bartlomieju for this work! One of the first substantial improvements to the "deno info" output since it was first created.

@bartlomieju - please create an issue for changing the json output to a flat graph structure.

KrisChambers closed this Jul 18, 2020

KrisChambers reopened this Jul 18, 2020

KrisChambers changed the title ~~Feature: Dependency count and module sizes for deno info.~~ Feature: Dependency count and module sizes for deno info. Feedback wanted. Jul 18, 2020

KrisChambers force-pushed the feature_add_dep_count_and_sizes_to_deno_info branch from 6a326da to c01b690 Compare July 22, 2020 17:14

KrisChambers force-pushed the feature_add_dep_count_and_sizes_to_deno_info branch 2 times, most recently from 72c9348 to 7681609 Compare July 24, 2020 02:36

KrisChambers marked this pull request as ready for review July 24, 2020 04:18

KrisChambers force-pushed the feature_add_dep_count_and_sizes_to_deno_info branch from 7681609 to c077316 Compare July 25, 2020 22:32

KrisChambers force-pushed the feature_add_dep_count_and_sizes_to_deno_info branch from c077316 to 1928a84 Compare July 28, 2020 17:57

bartlomieju added this to the 1.3.0 milestone Aug 3, 2020

bartlomieju requested changes Aug 11, 2020

View reviewed changes

cli/main.rs Outdated Show resolved Hide resolved

cli/main.rs Outdated Show resolved Hide resolved

cli/main.rs Outdated Show resolved Hide resolved

cli/main.rs Outdated Show resolved Hide resolved

cli/main.rs Outdated Show resolved Hide resolved

bartlomieju requested a review from ry August 11, 2020 09:27

bartlomieju modified the milestones: 1.3.0, 1.4.0 Aug 13, 2020

bartlomieju mentioned this pull request Aug 17, 2020

[WIP] Refactor the compiler #6894

Closed

KrisChambers added 3 commits August 17, 2020 09:08

Add function for getting size in bytes.

c6da66e

Modify print_file_info to output file sizes

2110ed8

Refactor into a graph transformation for better readability

35ad6dd

dedupe sibling deps

054e0d0

lucacasonato added 3 commits September 2, 2020 13:23

display total unique dep size

6a3a335

fix tests

9feca56

added * for already displayed leafs

9f9c6fe

lucacasonato and others added 4 commits September 4, 2020 15:37

fix tests

97e10cd

remove typechecking from info

4bf09fc

Merge branch 'master' into feature_add_dep_count_and_sizes_to_deno_info

7c40f44

cleanup

65b4834

bartlomieju added 2 commits September 4, 2020 18:30

Merge branch 'master' into feature_add_dep_count_and_sizes_to_deno_info

eccb7e7

fix tests

90938ad

bartlomieju added 4 commits September 5, 2020 11:05

fix test

99a89cd

Merge branch 'master' into feature_add_dep_count_and_sizes_to_deno_info

dfc554e

Merge branch 'master' into feature_add_dep_count_and_sizes_to_deno_info

05f0538

add test with compiled source

575fb9b

bartlomieju unassigned ry Sep 7, 2020

ry reviewed Sep 7, 2020

View reviewed changes

core/modules.rs

assert_eq!(bar_deps.deps, Some(vec![]));

}

*/

Copy link

Member

ry Sep 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great to see this moved out of core

ry approved these changes Sep 7, 2020

View reviewed changes

bartlomieju approved these changes Sep 7, 2020

View reviewed changes

bartlomieju merged commit 1cd2267 into denoland:master Sep 7, 2020

bartlomieju mentioned this pull request Sep 15, 2020

refactor(unstable): deno info --json output #7417

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(info): Dependency count and sizes #6786

feat(info): Dependency count and sizes #6786

KrisChambers commented Jul 17, 2020

cknight commented Jul 21, 2020

KrisChambers commented Jul 22, 2020

cknight commented Jul 22, 2020

KrisChambers commented Jul 24, 2020

SyrupThinker commented Jul 24, 2020

KrisChambers commented Jul 25, 2020

CLAassistant commented Jul 27, 2020 •

edited

Loading

bartlomieju commented Aug 3, 2020

KrisChambers commented Aug 5, 2020

ry commented Aug 12, 2020

KrisChambers commented Aug 13, 2020

bartlomieju commented Aug 13, 2020

bartlomieju commented Sep 2, 2020

lucacasonato commented Sep 4, 2020

bartlomieju commented Sep 4, 2020

bartlomieju commented Sep 4, 2020

bartlomieju commented Sep 7, 2020

ry Sep 7, 2020

ry left a comment

feat(info): Dependency count and sizes #6786

feat(info): Dependency count and sizes #6786

Conversation

KrisChambers commented Jul 17, 2020

cknight commented Jul 21, 2020

KrisChambers commented Jul 22, 2020

cknight commented Jul 22, 2020

KrisChambers commented Jul 24, 2020

SyrupThinker commented Jul 24, 2020

KrisChambers commented Jul 25, 2020

CLAassistant commented Jul 27, 2020 • edited Loading

bartlomieju commented Aug 3, 2020

KrisChambers commented Aug 5, 2020

ry commented Aug 12, 2020

KrisChambers commented Aug 13, 2020

bartlomieju commented Aug 13, 2020

bartlomieju commented Sep 2, 2020

lucacasonato commented Sep 4, 2020

bartlomieju commented Sep 4, 2020

bartlomieju commented Sep 4, 2020

bartlomieju commented Sep 7, 2020

ry Sep 7, 2020

Choose a reason for hiding this comment

ry left a comment

Choose a reason for hiding this comment

CLAassistant commented Jul 27, 2020 •

edited

Loading