New issue
Advanced search Search tips

Issue 789028 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Task



Sign in to add a comment

Better algorithm for managing event timeouts on service workers

Project Member Reported by shimazu@chromium.org, Nov 28 2017

Issue description

Context: https://chromium-review.googlesource.com/c/chromium/src/+/760083#74

Currently there are two data structures:
- a map from the event id to its abort callback
- a queue of a pair of timeout time and the event id

The entries in the queue for finished events couldn't be removed, so these entries could be remaining until the timeout time for each event (5 min). 

 
std::map seems good for that purpose.
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ad50bed3f641eaa1d28e88632a2f771765c3c7c0

commit ad50bed3f641eaa1d28e88632a2f771765c3c7c0
Author: Makoto Shimazu <shimazu@chromium.org>
Date: Wed Nov 29 10:37:39 2017

Use std::set for keeping events in ServiceWorkerTimeoutTimer

This patch removes timeout times for finished events from SWTimeoutTimer. This
work reduces the memory usage since we no longer have to keep all of timeout
times for 5 minutes.
The same fix can be applied to SWVersion::RequestInfo.

Bug:  789028 
Change-Id: Ie8f421307f8e0953372381d620a3736f076c64c2
Reviewed-on: https://chromium-review.googlesource.com/792771
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520065}
[modify] https://crrev.com/ad50bed3f641eaa1d28e88632a2f771765c3c7c0/content/renderer/service_worker/service_worker_timeout_timer.cc
[modify] https://crrev.com/ad50bed3f641eaa1d28e88632a2f771765c3c7c0/content/renderer/service_worker/service_worker_timeout_timer.h

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/31d13118eeec6d2baf2d14d6e43152b58b27c63a

commit 31d13118eeec6d2baf2d14d6e43152b58b27c63a
Author: Makoto Shimazu <shimazu@chromium.org>
Date: Thu Nov 30 05:20:58 2017

Use std::set for timeout times in ServiceWorkerVersion

Currently we are keeping the timeout times by a priority queue, but it has all
of entries until they expire. This patch removes an entry when the event has
finished. This is similar to crrev.com/c/792771.

Bug:  789028 
Change-Id: Ie869500c8385defd247b7ef81fe40da56cb57bfd
Reviewed-on: https://chromium-review.googlesource.com/795630
Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520449}
[modify] https://crrev.com/31d13118eeec6d2baf2d14d6e43152b58b27c63a/content/browser/service_worker/service_worker_version.cc
[modify] https://crrev.com/31d13118eeec6d2baf2d14d6e43152b58b27c63a/content/browser/service_worker/service_worker_version.h

Status: Fixed (was: Available)
Labels: M-64

Sign in to add a comment