720045 - simplify conservative stack scanner with single-threaded runtime

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect)

Description

[Igor Bukanov]

+++ This bug was initially created as a clone of Bug #675078 +++

With 675078 fixed we no longer need to record the native stack boundaries in the runtime. As the GC only need to deal with one thread and that thread is current one, we can always query for the native stack bounds directly during the GC.

Comment 1

[Luke Wagner [:luke]]

The patch in bug 675078 does this.

Comment 2

[Igor Bukanov]

(In reply to Luke Wagner [:luke] from comment #1)
> The patch in bug 675078 does this.

No, it is not - the patch still stores the base of the native thread in JSRuntime. It is not necessary.

Comment 3

[Igor Bukanov]

Attachment: v1

The patch always scans the native stack unless it is explicitly disabled via AutoDisableConservativeStackScan class. The latter is a temporary measure to keep the current logic in nsXPConnect::Collect. For now I want to keep this to make sure that in the current form the patch does not increase orange factor due to false positives roots.

To minimize the recorded stack depth the patch requires that any code that needs to scan the stack uses the callback to implements its logic. This way RecordNativeStackTopForGC can use the shallowest stack point when recording the stack. Again, this complication can be reverted if it does not cause extra orange, but for now the main goal is to decouple the conservative stack scanner from the JS request infrastructure.

Comment 4

[Igor Bukanov]

Comment on attachment 592878 [details] [diff] [review]
v1

Try server shows a leak with the patch

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

I suspect the conservative stack scanner is no longer of much interest at this point.

Comment 6

[Chris Peterson [:cpeterson]]

Generational GC (bug 619558) will obsolete the conservative stack scanner.