Skip to content
This repository has been archived by the owner on Dec 3, 2020. It is now read-only.

Improve background updates performance for a large number of tracked products #166

Closed
biancadanforth opened this issue Oct 15, 2018 · 1 comment
Assignees
Milestone

Comments

@biancadanforth
Copy link
Collaborator

biancadanforth commented Oct 15, 2018

Last week, @johngruen suggested we test the extension with a large number of tracked products to see what happens performance-wise when background updates are underway. Currently, we create an iframe for each tracked product without gating them one at a time.

I tracked 15 products and profiled the extension during background updates. Sure enough, the average frame rate was 18 fps, and extraction is taking up the lion's share of the time when frame rate drops precipitously (about 75% from one random sample, shown below).

screen shot 2018-10-15 at 10 51 26 am

One thing we could try is performing each update in series; waiting to perform the next update until the previous update has completed.

One probably post-MVP idea related to this would be to cache which element we originally extracted the price from and just try to pull that element from the page during background updates first before re-running Fathom completely.

@biancadanforth biancadanforth added the [ENG]: Triage Work the team needs to review to determine if it will be included as part of the next milestone. label Oct 15, 2018
@biancadanforth biancadanforth added this to the November MVP milestone Oct 15, 2018
@muccimoz
Copy link
Collaborator

Team reviewed and this issue is being added to the November Milestone.

@muccimoz muccimoz added mvp-November milestone and removed [ENG]: Triage Work the team needs to review to determine if it will be included as part of the next milestone. labels Oct 16, 2018
@biancadanforth biancadanforth self-assigned this Oct 18, 2018
biancadanforth added a commit that referenced this issue Oct 18, 2018
Previously, in the worst case scenario that a user adds a bunch (15+) of products at close to the same time, our background price checking would load several product pages each in its own iframe in the background page at once. This introduced a lot of jank (average FPS 14 for 15 tracked products added at the same time).

This patch ensures that when more than one product needs a price update check, that the list is queued up, so that only one product page is loaded into an iframe in the background page at a time (average FPS 45 for 15 tracked products added at the same time).

Notes:
* These FPS values are for a worst case scenario. Most users will not be adding new products to the list within a couple seconds of each other, so most of the time, the number of products in queue for a background price update should be closer to 0 than 15.
* For reference, a baseline FPS for 0 tracked products is 59 and for 1 tracked product is 54.
* It is technically possible to load more than one iframe at a time with this patch, however, that is only the case when `(n * iframeTimeout) > priceCheckInterval || (n * iframeTimeout > priceCheckTimeoutInterval). With the current default values for these config timeouts/intervals, the user would need to be tracking at least 90 items for this to occur.
* Here is the `web-ext-config.js` I used for testing this patch:

```javascript
module.exports = {
  run: {
    pref: [
      '[email protected]=45000',
      '[email protected]=45000',
      '[email protected]=8000',
    ],
    startUrl: [
      'https://www.amazon.com/KitchenAid-KL26M1XER-Professional-6-Qt-Bowl-Lift/dp/B01LYV1U30?smid=ATVPDKIKX0DER&pf_rd_p=0c7b792f-241a-4510-94f4-dd184a76f201&pf_rd_r=AZD7BGV3JZGTB23F30X3',
      'https://www.amazon.com/dp/B077YG22Y9/ref=sspa_dk_detail_3?psc=1&pd_rd_i=B077YG22Y9&pd_rd_wg=mfQm0&pd_rd_r=K27A84Z9Y01R47H7BJWR&pd_rd_w=km01K',
      'https://www.amazon.com/Lindt-Assorted-Chocolate-Gourmet-Truffles/dp/B004XDMS3C/ref=sr_1_6_s_it?s=grocery&ie=UTF8&qid=1533437720&sr=1-6&keywords=chocolates&th=1',
      'https://www.ebay.com/p/Best-Choice-Products-650W-6-speed-5-5QT-Kitchen-Food-Stand-Mixer-with-Stainless-Steels-Bowl-Black/3018375728?iid=253733404998',
      'https://www.ebay.com/itm/Philadelphia-Candies-Milk-Chocolate-Covered-Assorted-Nuts-1-Pound-Gift-Box/263103368447?epid=1301555428&hash=item3d422eccff',
      'https://www.ebay.com/itm/Dell-Inspiron-7570-15-6-Touch-Laptop-i7-8550U-1-8GHz-8GB-1TB-NVIDIA-940MX-W10/263827294291',
      'https://www.walmart.com/ip/KitchenAid-Classic-Series-4-5-Quart-Tilt-Head-Stand-Mixer-Onyx-Black-K45SSOB/29474640',
      'https://www.walmart.com/ip/HP-15-bs212wm-15-6-Laptop-Windows-10-Intel-Celeron-N4000-Processor-4GB-Memory-500GB-Hard-Drive-DVD-Jet-Black/139270579',
      'https://www.walmart.com/ip/Reese-s-Kit-Kat-Chocolate-Candy-Miniatures-Assortment-40-Oz/10449915',
      'https://www.bestbuy.com/site/kitchenaid-artisan-tilt-head-stand-mixer-ocean-drive/6133651.p?skuId=6133651',
      'https://www.bestbuy.com/site/swiss-miss-milk-chocolate-hot-cocoa-k-cup-pods-16-pack/4700838.p?skuId=4700838',
      'https://www.bestbuy.com/site/jbl-everest-elite-750nc-wireless-over-ear-noise-cancelling-headphones-gunmetal/5840136.p?skuId=5840136',
      'https://www.homedepot.com/p/KitchenAid-Classic-4-5-Qt-Tilt-Head-White-Stand-Mixer-K45SSWH/202546032',
      'https://www.homedepot.com/p/Flossugar-1-2-Gal-Chocolate-93216CT/302429246',
      'https://www.homedepot.com/p/Commercial-Electric-Around-the-Neck-Premium-Bluetooth-Stereo-Earbuds-HD0855/300372796',
      'about:debugging',
    ],
  },
};
```
biancadanforth added a commit that referenced this issue Oct 19, 2018
Previously, in the worst case scenario that a user adds a bunch (15+) of products at close to the same time, our background price checking would load several product pages each in its own iframe in the background page at once. This introduced a lot of jank (average FPS 14 for 15 tracked products added at the same time).

This patch ensures that when more than one product needs a price update check, that the list is queued up, so that only one product page is loaded into an iframe in the background page at a time (average FPS 45 for 15 tracked products added at the same time).

Notes:
* These FPS values are for a worst case scenario. Most users will not be adding new products to the list within a couple seconds of each other, so most of the time, the number of products in queue for a background price update should be closer to 0 than 15.
* For reference, a baseline FPS for 0 tracked products is 59 and for 1 tracked product is 54.
* It is technically possible to load more than one iframe at a time with this patch, however, that is only the case when `(n * iframeTimeout) > priceCheckInterval || (n * iframeTimeout > priceCheckTimeoutInterval). With the current default values for these config timeouts/intervals, the user would need to be tracking at least 90 items for this to occur.
* Here is the `web-ext-config.js` I used for testing this patch:

```javascript
module.exports = {
  run: {
    pref: [
      '[email protected]=45000',
      '[email protected]=45000',
      '[email protected]=8000',
    ],
    startUrl: [
      'https://www.amazon.com/KitchenAid-KL26M1XER-Professional-6-Qt-Bowl-Lift/dp/B01LYV1U30?smid=ATVPDKIKX0DER&pf_rd_p=0c7b792f-241a-4510-94f4-dd184a76f201&pf_rd_r=AZD7BGV3JZGTB23F30X3',
      'https://www.amazon.com/dp/B077YG22Y9/ref=sspa_dk_detail_3?psc=1&pd_rd_i=B077YG22Y9&pd_rd_wg=mfQm0&pd_rd_r=K27A84Z9Y01R47H7BJWR&pd_rd_w=km01K',
      'https://www.amazon.com/Lindt-Assorted-Chocolate-Gourmet-Truffles/dp/B004XDMS3C/ref=sr_1_6_s_it?s=grocery&ie=UTF8&qid=1533437720&sr=1-6&keywords=chocolates&th=1',
      'https://www.ebay.com/p/Best-Choice-Products-650W-6-speed-5-5QT-Kitchen-Food-Stand-Mixer-with-Stainless-Steels-Bowl-Black/3018375728?iid=253733404998',
      'https://www.ebay.com/itm/Philadelphia-Candies-Milk-Chocolate-Covered-Assorted-Nuts-1-Pound-Gift-Box/263103368447?epid=1301555428&hash=item3d422eccff',
      'https://www.ebay.com/itm/Dell-Inspiron-7570-15-6-Touch-Laptop-i7-8550U-1-8GHz-8GB-1TB-NVIDIA-940MX-W10/263827294291',
      'https://www.walmart.com/ip/KitchenAid-Classic-Series-4-5-Quart-Tilt-Head-Stand-Mixer-Onyx-Black-K45SSOB/29474640',
      'https://www.walmart.com/ip/HP-15-bs212wm-15-6-Laptop-Windows-10-Intel-Celeron-N4000-Processor-4GB-Memory-500GB-Hard-Drive-DVD-Jet-Black/139270579',
      'https://www.walmart.com/ip/Reese-s-Kit-Kat-Chocolate-Candy-Miniatures-Assortment-40-Oz/10449915',
      'https://www.bestbuy.com/site/kitchenaid-artisan-tilt-head-stand-mixer-ocean-drive/6133651.p?skuId=6133651',
      'https://www.bestbuy.com/site/swiss-miss-milk-chocolate-hot-cocoa-k-cup-pods-16-pack/4700838.p?skuId=4700838',
      'https://www.bestbuy.com/site/jbl-everest-elite-750nc-wireless-over-ear-noise-cancelling-headphones-gunmetal/5840136.p?skuId=5840136',
      'https://www.homedepot.com/p/KitchenAid-Classic-4-5-Qt-Tilt-Head-White-Stand-Mixer-K45SSWH/202546032',
      'https://www.homedepot.com/p/Flossugar-1-2-Gal-Chocolate-93216CT/302429246',
      'https://www.homedepot.com/p/Commercial-Electric-Around-the-Neck-Premium-Bluetooth-Stereo-Earbuds-HD0855/300372796',
      'about:debugging',
    ],
  },
};
```
biancadanforth added a commit that referenced this issue Oct 22, 2018
Previously, in the worst case scenario that a user adds a bunch (15+) of products at close to the same time, our background price checking would load several product pages each in its own iframe in the background page at once. This introduced a lot of jank (average FPS 14 for 15 tracked products added at the same time).

This patch ensures that when more than one product needs a price update check, that the list is queued up, so that only one product page is loaded into an iframe in the background page at a time (average FPS 45 for 15 tracked products added at the same time).

Notes:
* These FPS values are for a worst case scenario. Most users will not be adding new products to the list within a couple seconds of each other, so most of the time, the number of products in queue for a background price update should be closer to 0 than 15.
* For reference, a baseline FPS for 0 tracked products is 59 and for 1 tracked product is 54.
* It is technically possible to load more than one iframe at a time with this patch, however, that is only the case when `(n * iframeTimeout) > priceCheckInterval || (n * iframeTimeout > priceCheckTimeoutInterval). With the current default values for these config timeouts/intervals, the user would need to be tracking at least 90 items for this to occur.
* Here is the `web-ext-config.js` I used for testing this patch:

```javascript
module.exports = {
  run: {
    pref: [
      '[email protected]=45000',
      '[email protected]=45000',
      '[email protected]=8000',
    ],
    startUrl: [
      'https://www.amazon.com/KitchenAid-KL26M1XER-Professional-6-Qt-Bowl-Lift/dp/B01LYV1U30?smid=ATVPDKIKX0DER&pf_rd_p=0c7b792f-241a-4510-94f4-dd184a76f201&pf_rd_r=AZD7BGV3JZGTB23F30X3',
      'https://www.amazon.com/dp/B077YG22Y9/ref=sspa_dk_detail_3?psc=1&pd_rd_i=B077YG22Y9&pd_rd_wg=mfQm0&pd_rd_r=K27A84Z9Y01R47H7BJWR&pd_rd_w=km01K',
      'https://www.amazon.com/Lindt-Assorted-Chocolate-Gourmet-Truffles/dp/B004XDMS3C/ref=sr_1_6_s_it?s=grocery&ie=UTF8&qid=1533437720&sr=1-6&keywords=chocolates&th=1',
      'https://www.ebay.com/p/Best-Choice-Products-650W-6-speed-5-5QT-Kitchen-Food-Stand-Mixer-with-Stainless-Steels-Bowl-Black/3018375728?iid=253733404998',
      'https://www.ebay.com/itm/Philadelphia-Candies-Milk-Chocolate-Covered-Assorted-Nuts-1-Pound-Gift-Box/263103368447?epid=1301555428&hash=item3d422eccff',
      'https://www.ebay.com/itm/Dell-Inspiron-7570-15-6-Touch-Laptop-i7-8550U-1-8GHz-8GB-1TB-NVIDIA-940MX-W10/263827294291',
      'https://www.walmart.com/ip/KitchenAid-Classic-Series-4-5-Quart-Tilt-Head-Stand-Mixer-Onyx-Black-K45SSOB/29474640',
      'https://www.walmart.com/ip/HP-15-bs212wm-15-6-Laptop-Windows-10-Intel-Celeron-N4000-Processor-4GB-Memory-500GB-Hard-Drive-DVD-Jet-Black/139270579',
      'https://www.walmart.com/ip/Reese-s-Kit-Kat-Chocolate-Candy-Miniatures-Assortment-40-Oz/10449915',
      'https://www.bestbuy.com/site/kitchenaid-artisan-tilt-head-stand-mixer-ocean-drive/6133651.p?skuId=6133651',
      'https://www.bestbuy.com/site/swiss-miss-milk-chocolate-hot-cocoa-k-cup-pods-16-pack/4700838.p?skuId=4700838',
      'https://www.bestbuy.com/site/jbl-everest-elite-750nc-wireless-over-ear-noise-cancelling-headphones-gunmetal/5840136.p?skuId=5840136',
      'https://www.homedepot.com/p/KitchenAid-Classic-4-5-Qt-Tilt-Head-White-Stand-Mixer-K45SSWH/202546032',
      'https://www.homedepot.com/p/Flossugar-1-2-Gal-Chocolate-93216CT/302429246',
      'https://www.homedepot.com/p/Commercial-Electric-Around-the-Neck-Premium-Bluetooth-Stereo-Earbuds-HD0855/300372796',
      'about:debugging',
    ],
  },
};
```
biancadanforth added a commit that referenced this issue Oct 22, 2018
Fix #166: Load background iframes for price updates one at a time
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants