Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

progress-bar support #879

Closed
JohnAllen opened this issue Jun 10, 2017 · 14 comments
Closed

progress-bar support #879

JohnAllen opened this issue Jun 10, 2017 · 14 comments
Assignees
Labels
P2 Medium: Good to have, but can wait until someone steps up status/in-progress In progress

Comments

@JohnAllen
Copy link

I'd like to take a stab at implementing progress-bar support for js-ipfs. Any pointers and/or preferred implementation approaches? E.g., packages or homebrewed solutions?

@JohnAllen
Copy link
Author

JohnAllen commented Jun 10, 2017

@diasdavid already referenced a related js-ipfs-unixfs-engine issue.

@daviddias
Copy link
Member

Woot! Thank you @JohnAllen :D

With regards to implementation, as long as it provides the information necessary using the same pattern that go-ipfs uses, so that we can expose that through the http-api and use js-ipfs-api as a client, we are golden. Of course this will lead you to change a bit the ipfs-unixfs-engine, but that is expected.

With regards to UI, there is this list of awesome CLI utilities -- https://github.com/sindresorhus/awesome-nodejs#command-line-utilities -- which you can take inspiration from. One that seems to get this job right is: https://github.com/visionmedia/node-progress

Another note. As you implement this through the daemon, you will hit #823 as well. Consider implementing first all of what is required to have the progress bar working while offline and then fixing the buffering of files to be sent out.

@JohnAllen
Copy link
Author

JohnAllen commented Jun 11, 2017

Cool appreciate the tips. And by "ipfs-unixfs-engine" you actually mean the "js-ipfs-unixfs-engine", correct? I ask because there isn't an "ipfs-unixfs-engine" repo.

@daviddias
Copy link
Member

@JohnAllen that is correct :)

@pgte
Copy link
Contributor

pgte commented Jun 12, 2017

@JohnAllen js-ipfs-unixfs-engine implements a duplex stream, where the input has the individual files (represented by a path and the individual content as streams) and outputs the hash, path and node size for each file. So using the unixfs output stream you already have a notion of progress (either through the number of files or their relative size).

What you now need is the total size (either number of files and / or total size), which you can only compute by having direct access to the filesystem. This means that js-ipfs-unixfs-engine can't help you with that.

I think that the best place to figure get the total size before starting pumping the contents into the unixfs stream is in js-ipfs itself, namely on src/cli/commands/files/add.js.

Hope this helps. Ping me if you need help.

@pgte
Copy link
Contributor

pgte commented Jun 12, 2017

@JohnAllen Been reading through ipfs-inactive/js-ipfs-unixfs-engine#123, and there's a good point:

The emitted results contain the IPFS total object size and not contain the original file size. This needs to be solved first. Hit me if you need pointers in js-ipfs.unixfs-engine for that.

@JohnAllen
Copy link
Author

@pgte Mind pointing me to where the object size is emitted in js-ipfs-unixfs-engine? Think I can do the rest just not sure where that is. Thanks!

@pgte
Copy link
Contributor

pgte commented Jun 26, 2017

TLDR: it looks like, if I'm reading the code right that, after all, you can use the file size output to calculate the total file output without changing unixfs.

Right now, the emitted file contains the file size:
https://github.com/ipfs/js-ipfs-unixfs-engine/blob/master/src/exporter/file.js#L43

The size is computed in the file.fileSize() method, which sums the total of the block sizes.

When the file is a single DAG node, the fileSize() contains the size of the data, which is the correct file size.

When the file is composed by more than one DAG node (for bigger files), the block sizes are computed from all the leave sizes (a file may be more than one node) here, which in turn are computed from the individual node file size.

Please tell me if you experience otherwise.

@JohnAllen
Copy link
Author

FYI working on this. Hope to PR this week. Have simple numFiles based approach working but will work on basing off of fileSize soon.

@daviddias daviddias added status/ready Ready to be worked status/in-progress In progress and removed status/ready Ready to be worked labels Jul 3, 2017
@daviddias daviddias changed the title Progress-Bar Support Discussion progress-bar support Jul 3, 2017
@JohnAllen
Copy link
Author

Hey everyone, did some work on this over the weekend. Thanks for being so patient with me.

To use a byte-based approach of showing progress in a progress bar, we'll have to iterate over every file before the addStream begins (unless there is a way to do that) to know the total number of bytes. Yes we get the fileSize() for each individual file in the above method as we're streaming the files through, but do we have the total number of bytes prior to this? Doesn't seem like it to me. glob is what gets our directories' files -- would be cool if glob included total file size but I didn't see that in its readme.

I wanted to check before I make this change. It seems to me that a pure numFiles approach may suffice: large files will take longer and progress' approach of calculating ETA will include the fact that a large file is being processed (because elapsed in this line will be larger).

Also, adding is so fast for a small number of files that the progress bar is not even visible. It works but the filename and hash output begins very quickly (this is for small files, yes).

Unfortunately I haven't been able to test a large number of files (say greater than 500 mostly empty .txt files), nor large files ( even just a few MB image files) because I keep getting seg faults and Bus error: 10 with the daemon. That is with the most recent versions after I reinstalled everything this weekend (hate to be the bearer of bad news :( )

Anyways, let me know how you'd like to proceed. I should probably push what I have so you can glance at it -- will do that shortly.

@daviddias
Copy link
Member

Thank you for the update, @JohnAllen !

To use a byte-based approach of showing progress in a progress bar, we'll have to iterate over every file before the addStream begins (unless there is a way to do that) to know the total number of bytes.

This seems a bit overkill, a simpler solution would be:

    1. Check the filesize
    1. Every time a chunk is added (256KiB) we update the progress bar status

You don't need to count the bytes that have been sent, you just need to count the bytes that have been chunked, hashed and stored.

glob is what gets our directories' files -- would be cool if glob included total file size but I didn't see that in its readme.

Yes, this might have to be added on that step. You can add a fs.stats in the addPipeline https://github.com/ipfs/js-ipfs/blob/master/src/cli/commands/files/add.js#L116

Unfortunately I haven't been able to test a large number of files (say greater than 500 mostly empty .txt files), nor large files ( even just a few MB image files) because I keep getting seg faults and Bus error: 10 with the daemon.

I just added more than one 700MB file and one with 1.2GB and it finished without any error. Both with daemon on and off. Are you sure you are using latest jsipfs? Which OS do you use? Could you do a npm fresh install?

@daviddias daviddias added the P2 Medium: Good to have, but can wait until someone steps up label Jul 26, 2017
@JohnAllen
Copy link
Author

Hey @diasdavid bummed I haven't been able to submit something for this. Hopefully sometime in the future I can contribute elsewhere.

@bmordan
Copy link
Contributor

bmordan commented Sep 4, 2017

Here is a PR in progress for a progress bar tableflip#2

@daviddias
Copy link
Member

Now happening on #994, let's keep tracking it there :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P2 Medium: Good to have, but can wait until someone steps up status/in-progress In progress
Projects
None yet
Development

No branches or pull requests

4 participants