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

Performance of window.ipfs #460

Closed
1 of 2 tasks
lidel opened this issue Apr 16, 2018 · 6 comments
Closed
1 of 2 tasks

Performance of window.ipfs #460

lidel opened this issue Apr 16, 2018 · 6 comments
Labels
area/window-ipfs Issues related to IPFS API exposed on every page help wanted Seeking public contribution on this issue status/in-progress In progress

Comments

@lidel
Copy link
Member

lidel commented Apr 16, 2018

Content script is injected into every webpage, which comes at a price:

  • add-on/dist/contentScripts/ipfs-proxy/ is ~2.8MB

This raises some questions:

  • what is the performance impact on general browsing experience?
  • how can we make it better?

cc @alanshaw


Status

@lidel lidel added help wanted Seeking public contribution on this issue area/window-ipfs Issues related to IPFS API exposed on every page labels Apr 16, 2018
@alanshaw
Copy link
Member

I'll measure where the bottleneck is, but some ideas:

  1. We include the IPFS constructor in the page, which is ~3MB of JS that needs to be read and parsed by the browser (this was the reason for the issue with the dropdown being slow to appear).
  2. All pages open a messaging "port" to the background process, even if it's not used - this could be done lazily
  3. We have all_frames: true which means that all frames of a site get a window.ipfs. Imagine performance issues of 1 & 2 but 10x because the site has loads of adverts that are served in an iframe for example

My hunch is that we can mitigate 3 by solving 1 and/or 2.

@alanshaw
Copy link
Member

My findings are as follows:

On my circa 2014 MacBook Pro the content script currently take the following time to run:

  • ~1,100ms (Firefox)
  • ~900ms (Chrome)

If I remove window.Ipfs:

  • ~500ms (Firefox)
  • ~300ms (Chrome)

Which is >50% time saving.

The problem is exacerbated on sites what have many iframes. For example, http://theregister.co.uk had 12 iframes when I ran the tests.

The content scripts are run in serial, i.e. the next one does not start until the current one is finished. It means that I can actually see the adverts load in one after the other.

I believe the time is being spent parsing the IPFS library - it's nearly 3MB of JS.

My recommendation would be to remove adding the constructor window.Ipfs.

N.B. I experimented with lazily establishing the IPC port from the content script to the background script but it has no significant effect on run time.

Also, I checked with minified scripts and it also doesn't have a significant effect on run time.

lidel added a commit that referenced this issue Apr 25, 2018
@lidel
Copy link
Member Author

lidel commented Apr 25, 2018

Thank you for looking into this!

My recommendation would be to remove adding the constructor window.Ipfs

I agree. It looks like it is barely used anyway.
If needed, developer can always just include it via <script>.

Some additional data for add-on/dist/contentScripts/ipfs-proxy/ with window.Ipfs:

-rw-r--r-- 1 lidel lidel 2.8M Apr 23 11:15 content.js

And without window.Ipfs:

-rw-r--r-- 1 lidel lidel 1.1M Apr 25 12:00 content.js

That is ~1.7MB difference (60%!)

Will create PR shortly. Edit: PR is at #467

N.B. I experimented with lazily establishing the IPC port from the content script to the background script but it has no significant effect on run time.

Just to clarify: no at all, or no effect on load time? Do we know what is impact on memory usage? I would suspect that every time extension is establishing a new port some memory goes away, and if we can avoid that, that would be great.

@alanshaw
Copy link
Member

alanshaw commented Apr 25, 2018

Just to clarify: no at all, or no effect on load time?

I haven't taken any memory measurements, but we should probably do lazy IPC port anyway as there is an overhead.

@lidel lidel added the status/in-progress In progress label Apr 25, 2018
@lidel lidel added this to the v2.3.0 milestone Apr 25, 2018
@lidel
Copy link
Member Author

lidel commented Jul 7, 2018

A note from #webextensions

Will: thinking about how executeScript probably works internally (cross-process messaging), it might be more efficient to reduce your number of executeScript calls to just a module loader script that could then load all your other scripts in parallel whilst ensuring they execute in the correct order

@lidel lidel removed this from the v2.4.0 milestone Jul 7, 2018
@lidel
Copy link
Member Author

lidel commented Sep 20, 2018

Q3 summary:

@lidel lidel closed this as completed Sep 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/window-ipfs Issues related to IPFS API exposed on every page help wanted Seeking public contribution on this issue status/in-progress In progress
Projects
None yet
Development

No branches or pull requests

2 participants