New issue
Advanced search Search tips

Issue 758721 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Remove ReadWriteLock

Project Member Reported by robliao@chromium.org, Aug 24 2017

Issue description

There are only two users of ReadWriteLock in just over a year. Both are used to synchronize some sort of shutdown.

There are also non-trivial performance implications in using an SRWLock on Windows in shared mode. 

Discussion:
https://groups.google.com/a/chromium.org/d/topic/chromium-dev/cPZKCTpVw4U/discussion
 

Comment 1 Deleted

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 26 2017

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

commit 94fef6e2a56ef5b3ed0dc0fd94e6ad52267067fb
Author: Robert Liao <robliao@chromium.org>
Date: Sat Aug 26 01:40:35 2017

Change ReadWriteLock to Lock+ConditionVariable in TaskService

There are non-trivial performance implications of using shared
SRWLocking on Windows as more state has to be checked.

Since there are only two uses of the ReadWriteLock in Chromium after
over 1 year, the decision is to remove it.

BUG= 758721 

Change-Id: I84d1987d7b624a89e896eb37184ee50845c39d80
Reviewed-on: https://chromium-review.googlesource.com/634423
Commit-Queue: Robert Liao <robliao@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Francois Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497632}
[modify] https://crrev.com/94fef6e2a56ef5b3ed0dc0fd94e6ad52267067fb/media/midi/task_service.cc
[modify] https://crrev.com/94fef6e2a56ef5b3ed0dc0fd94e6ad52267067fb/media/midi/task_service.h

Project Member

Comment 3 by bugdroid1@chromium.org, Sep 12 2017

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

commit d301af71fa12496dd9ab550454ed36647ac85af2
Author: Robert Liao <robliao@chromium.org>
Date: Tue Sep 12 23:07:27 2017

Remove ReadWriteLock from IncomingTaskQueue

Equivalent functionality can be achieved by adding a bool without any
additional locking in the IncomingTaskQueue::PostPendingTask path.

One fewer lock is required if there is no need to schedule work.

BUG=749312, 758721 

Change-Id: I812db615ef2d1772c077266240d3a1927c97565c
Reviewed-on: https://chromium-review.googlesource.com/639158
Commit-Queue: Robert Liao <robliao@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Francois Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501441}
[modify] https://crrev.com/d301af71fa12496dd9ab550454ed36647ac85af2/base/message_loop/incoming_task_queue.cc
[modify] https://crrev.com/d301af71fa12496dd9ab550454ed36647ac85af2/base/message_loop/incoming_task_queue.h

Status: Fixed (was: Started)

Sign in to add a comment