simplify conservative stack scanner with single-threaded runtime

RESOLVED WONTFIX

Status

()

Core
JavaScript Engine
RESOLVED WONTFIX
5 years ago
4 years ago

People

(Reporter: Igor Bukanov, Assigned: Igor Bukanov)

Tracking

(Blocks: 2 bugs)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
+++ 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

5 years ago
The patch in bug 675078 does this.
(Assignee)

Comment 2

5 years ago
(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.
(Assignee)

Updated

5 years ago
Blocks: 722345
(Assignee)

Comment 3

5 years ago
Created attachment 592878 [details] [diff] [review]
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.
Assignee: general → igor
(Assignee)

Updated

5 years ago
Attachment #592878 - Flags: review?(anygregor)
(Assignee)

Comment 4

5 years ago
Comment on attachment 592878 [details] [diff] [review]
v1

Try server shows a leak with the patch
Attachment #592878 - Flags: review?(anygregor)
I suspect the conservative stack scanner is no longer of much interest at this point.
Generational GC (bug 619558) will obsolete the conservative stack scanner.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.