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

Notes on Cache and Performance #730

Closed
hacdias opened this issue Jul 19, 2018 · 10 comments
Closed

Notes on Cache and Performance #730

hacdias opened this issue Jul 19, 2018 · 10 comments

Comments

@hacdias
Copy link
Member

hacdias commented Jul 19, 2018

So, we're building the new WebUI and we want it to be amazingly good and blazing fast. Some of the last changes introduced, such as the #702 and #718 in the future, can slow down the pages a bit (at least on my computer on Chrome).

Why? #702 because it is always fetching the locations for our peers. I have 800+ in this moment so as soon as I open the WebUI what happens? It starts fetching all of them, even when we're not on the peers page, with a maximum of 10 concurrently. During that time, opening the File Browser to add files or even just navigate can be a pain. After having the info about all peers, it gets normal.

On #718 the same thing would happen.

I've got two different suggestions that could be applied to solve this issues.

1 Caching

Why not cache the peer location's information for x days and then update it? Why not cache a file's health for y hours before updating it again? Those are not really node-dependent informations so it wouldn't matter much. It wouldn't harm anyone, and we'd have a more fluid WebUI. We could also provide some option to clean the cache or tell the user that there is a cache that can be cleaned through the browser.

Uh, and we could also cache information on Explore panel. After all, a hash is linked to a content. But I'm not sure if IPFS doesn't do this already internally when we fetch stuff from the DAG api. I still don't know how it will work for other IPLD things (like Git and Ethereum). Correct me if I'm wrong.

Cool links:

2 Web Workers

We could perform secondary data fetching in Web Workers to make the interface more responsive while we're fetching peers, peer informations and other background tasks that aren't crucially needed.

Some background fetchings we could make:

  • Peers
  • Peer Locations
  • Files find provs

Some things done in the main process:

  • Files stuff
  • Config stuff
  • Everything else absolutely needed for a specific page.

So, this is it. I'd personally like to take a shot at the caching stuff if you think it's a great idea. /cc @ipfs-shipyard/gui

@olizilla
Copy link
Member

I'm very much in favour of this. It's on the roadmap, we just wanted to hit the problems first before adding caching... having IPFS running on the same machine we the webui should make lots of look ups so fast that we don't need caching... but the api for peers is so low-level that we end up having to do a lot of work at the app layer. The longer term fix that @alanshaw is thinking about is to propose a new peers api that does the work of tracking peers bandwidth over time and bandwidth usage by root CID.

But I totally agree with you @hacdias we want webui to be awesome today, and I'm super happy that you've dug into the the redux-bundler docs and recommendations for this!

I'd pause on web-workers while we get the caching story figured out. I think part of that will be replacing the redux-bundler asyncResourceBundle with our own fork that takes into account that the IPFS api is often running on the same machine... an ipfsResouceBundle perhaps that know about content Ids and peer ids, and automatically caches things forever where they are content addressed.

@hacdias
Copy link
Member Author

hacdias commented Jul 20, 2018

Thanks @olizilla! 😄 Did you have the opportunity to check #731? Although I now had an idea: cache on MFS instead of the browser. What do you think? 😄I'd like to work on that. Not that automatic as you are saying but something in the lines of what I've done on #731 but with MFS instead.

@lidel
Copy link
Member

lidel commented Jul 20, 2018

having IPFS running on the same machine we the webui should make lots of look ups so fast that we don't need caching

I hate to be that guy, but it is a very risky assumption: there will be users running it against HTTP API over SSH Socks5 tunnel to a remote VPS[1] and updates will stop being instant. 🙃

It should be easy to simulate it via network-throttling-tool in Firefox. I agree with @hacdias, it will be suuper useful to identify biggest pain points with it and cache low hanging fruits first. 👍 It may be that some operations over slow connection lock the UI, and that is where Web Workers come in.

I feel in the long term we should aim at making UI work fine when loaded over "Good 2G" connection (eg. someone tethering 2G internet to a laptop to inspect a DAG, download a file from MFS or check node health).


[1] I've seen it before with HTTP API and Companion: such node looks like a localhost one, so you cant even detect and account for it being slow in without additional test heuristics in code

@olizilla
Copy link
Member

@hacdias you mean #732 ? I will check it out shortly!

@lidel good point! Right now were using asyncResourceBundle which is optimised for treating data from a remote api as precious jewels that must be cared for. We can continue doing that to keep thing fast even when access to the node is slow. There are things like, flags for "offline check" which by default has it skip making api requests if the browser reports that their is no connectivity to the internet, which we ignore here as we definitely do want to talk to our IPFS node even if it the browser things it's isolated.

I think we can have the best of both and add smarts to cache resources that are content addressed and treat the other info from the API as if it were a probably stale snapshot (from a possibly remote endoint) of how things were when we asked, and that we'll keep around as the best answer we have until the next reaponse. I think this is what @hacdias is working on, and I agree it's best if we don't change things to assume we'll get fast responses from the api.

@hacdias
Copy link
Member Author

hacdias commented Jul 20, 2018

@olizilla yes #732 😄

@hacdias
Copy link
Member Author

hacdias commented Jul 20, 2018

Uh, and taking into account what @lidel said about a remote node I think I can answer myself about this:

Although I now had an idea: cache on MFS instead of the browser. What do you think? 😄I'd like to work on that.

It would just be the same problem if the node was far away or the connection was slow. The best bet is browser cache. Right? @olizilla ping me when you see #732. I'd like to merge it and implement it on #718 too!

@alanshaw
Copy link
Member

Spin up a js-ipfs in the browser and store it in MFS there? 😛 ...right?

@hacdias
Copy link
Member Author

hacdias commented Jul 20, 2018

@alanshaw that would also be a good idea (I think). It could be a storage-only node. Do you think it would be a good idea?

@hacdias
Copy link
Member Author

hacdias commented Jul 20, 2018

Due to all of the background tasks WebUI is doing, reactX functions are not being called as frequently as they should or at least they're taking more time to be called due to blocking.

Changing reactFilesFetch to have those two console.logs:

bundle.reactFilesFetch = createSelector(
  'selectFilesIsLoading',
  'selectFilesShouldUpdate',
  'selectIpfsReady',
  'selectRouteInfo',
  'selectFiles',
  (isLoading, shouldUpdate, ipfsReady, {url, params}, files) => {
    if (!isLoading && ipfsReady && url.startsWith('/files')) {
      console.log('1')
      if (shouldUpdate || !files || files.path !== decodeURIComponent(params.path)) {
        console.log('2')
        return { actionCreator: 'doFetchFiles' }
      }
    }
  }
)

Provides this effect: https://youtu.be/1RQdA9YMBM8 . I had to wait about 5 seconds to call the function which is visibly slow. And this wasn't the slowest thing I've seen here. I'll probably try changing the FilesPage in order to call directly doFilesFetch instead of having this react function.

hacdias added a commit that referenced this issue Aug 1, 2018
Taking in account what I said on #730, I tried to do a little experiment with Peer Locations. It doesn't add much more code and it is clean and improves performance over time. I set the peer location cached for a week, but maybe we could set it to expire in more time or even don't expire at all.

It uses IndexedDB, which in case of not being available in that browser, just fails silently. This cache is supposed to be something to improve the app and not really required to be used.

License: MIT
Signed-off-by: Henrique Dias <hacdias@gmail.com>
@olizilla olizilla removed the revamp label Sep 27, 2018
@olizilla
Copy link
Member

We went with caching. There is more to do, but this issue has done it's job.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants