[oilpan]: Don't sweep while in safepoint.

[oilpan]: Don't sweep while in safepoint.

Related to the test failures seen in the worker-data-view-test.html test I think this will fix the issue. Basically postpone the sweep until after we have left the safepoint. I am still testing/trying to reproduce.

R=ager@chromium.org, erik.corry@gmail.com, haraken@chromium.org, oilpan-reviews@chromium.org, sof@opera.com, tkent@chromium.org, vegorov@chromium.org, zerny@chromium.org
BUG=

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=177124

[haraken, 2014-06-26 13:52:11]

https://codereview.chromium.org/351253002/diff/1/Source/platform/heap/ThreadS...
File Source/platform/heap/ThreadState.cpp (right):

https://codereview.chromium.org/351253002/diff/1/Source/platform/heap/ThreadS...
Source/platform/heap/ThreadState.cpp:201: state->setSweepRequested();

Do we need to call this?

I guess setSweepRequested is alerady called for all attached threads by the
GCing thread (see Heap::prepareForGC()).

[Mads Ager (chromium), 2014-06-26 13:52:40]

LGTM

https://codereview.chromium.org/351253002/diff/1/Source/platform/heap/ThreadS...
File Source/platform/heap/ThreadState.cpp (right):

https://codereview.chromium.org/351253002/diff/1/Source/platform/heap/ThreadS...
Source/platform/heap/ThreadState.cpp:201: state->setSweepRequested();
I don't think we need to actually set sweep requested here that should have been
already done otherwise the performPendingSweep call would do nothing here.

So it seem you can just remove performPendingSweep from here and call it after
checkAndPark.

[haraken, 2014-06-26 13:53:32]

LGTM2 with the comment.

[wibling-chromium, 2014-06-27 10:07:48]

Thanks for the reviews!

https://codereview.chromium.org/351253002/diff/1/Source/platform/heap/ThreadS...
File Source/platform/heap/ThreadState.cpp (right):

https://codereview.chromium.org/351253002/diff/1/Source/platform/heap/ThreadS...
Source/platform/heap/ThreadState.cpp:201: state->setSweepRequested();
On 2014/06/26 13:52:11, haraken wrote:
> 
> Do we need to call this?
> 
> I guess setSweepRequested is alerady called for all attached threads by the
> GCing thread (see Heap::prepareForGC()).

No, that is actually wrong. We risk sweeping when no marking has been done. Will
remove.

https://codereview.chromium.org/351253002/diff/1/Source/platform/heap/ThreadS...
Source/platform/heap/ThreadState.cpp:201: state->setSweepRequested();
On 2014/06/26 13:52:40, Mads Ager (chromium) wrote:
> I don't think we need to actually set sweep requested here that should have
been
> already done otherwise the performPendingSweep call would do nothing here.
> 
> So it seem you can just remove performPendingSweep from here and call it after
> checkAndPark.

Done.

[wibling-chromium, 2014-06-27 10:23:53]

The CQ bit was checked by wibling@chromium.org

[commit-bot: I haz the power, 2014-06-27 10:24:49]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wibling@chromium.org/351253002/20001

[commit-bot: I haz the power, 2014-06-27 20:29:58]

Change committed as 177124

[tkent, 2014-06-30 12:18:49]

On 2014/06/27 20:29:58, I haz the power (commit-bot) wrote:
> Change committed as 177124

It seems this CL was fatal to Web Database.  

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40...

A context thread waits for a database thread completion in SafePointScope.

void TaskSynchronizer::waitForTaskCompletion()
{
    if (ThreadState::current()) {
        // Prevent the deadlock between park request by other threads and
blocking
        // by m_synchronousCondition.
        ThreadState::SafePointScope scope(ThreadState::HeapPointersOnStack);
        waitForTaskCompletionInternal();
    } else {
        // If this thread is already detached, we no longer need to enter a safe
point scope.
        waitForTaskCompletionInternal();
    }
}

[wibling-chromium, 2014-06-30 12:54:51]

On 2014/06/30 12:18:49, tkent wrote:
> On 2014/06/27 20:29:58, I haz the power (commit-bot) wrote:
> > Change committed as 177124
> 
> It seems this CL was fatal to Web Database.  
> 
>
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40...
> 
> A context thread waits for a database thread completion in SafePointScope.
> 
> void TaskSynchronizer::waitForTaskCompletion()
> {
>     if (ThreadState::current()) {
>         // Prevent the deadlock between park request by other threads and
> blocking
>         // by m_synchronousCondition.
>         ThreadState::SafePointScope scope(ThreadState::HeapPointersOnStack);
>         waitForTaskCompletionInternal();
>     } else {
>         // If this thread is already detached, we no longer need to enter a
safe
> point scope.
>         waitForTaskCompletionInternal();
>     }
> }

That's odd. I just removed the performPendingSweep out until we are not under
the m_safepoint bool. Do you have any idea why the database thread is not being
signaled?