Cleanups for PrivateScriptData / SharedScriptData

RESOLVED FIXED

Status

()

enhancement
P2
normal
RESOLVED FIXED
2 months ago
2 months ago

People

(Reporter: tcampbell, Assigned: tcampbell)

Tracking

(Blocks 2 bugs)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Assignee

Description

2 months ago

These patches do cleanup around PrivateScriptData / SharedScriptData lifetime to make it easier to reason about error paths are dropping data during relazification as proposed in Bug 1529456.

  • Use RefPtr / UniquePtr for the references to these types instead of manual code. This also better documents things in JSScript.
  • Add an explicit constructor to SharedScriptData similar to PrivateScriptData
  • Delete the default copy constructors since these types have trailing data.
  • Define missing default field initializers for consistency
  • Add a GCManagedDeletePolicy for PrivateScriptData to be deleted outside of sweeping.
Assignee

Comment 1

2 months ago

Also add a GCManagedDeletePolicy so that the script data can be dropped
even when we are not sweeping.

Assignee

Comment 2

2 months ago

Use a GCManagedDeletePolicy for PrivateScriptData to allow deleting
PrivateScriptData outside of initialization/finalization. Also add an
empty finalize for use and to make explicit the difference between
early-destruction and gc-finalization.

Depends on D22715

Assignee

Comment 3

2 months ago

This is based on PrivateScriptData approach. We make sure to call all
default constructors even if the compiler will optimize away.

Depends on D22716

Assignee

Comment 4

2 months ago

Leave manual refcounting in the ScriptDataTable for now since it
requires a bit of care to make the automatic types do the right thing
when sweeping.

Depends on D22717

Attachment #9049534 - Attachment is obsolete: true
Assignee

Updated

2 months ago
Keywords: leave-open

Comment 5

2 months ago
Pushed by tcampbell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/35825a8005ab
Cleanups in PrivateScriptData. r=jandem
https://hg.mozilla.org/integration/autoland/rev/d5eff5c34ce9
Use RefPtr for SharedScriptData pointers. r=jandem

Comment 6

2 months ago
Backout by dluca@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/638b7cfb8639
Backed out 2 changesets build bustage. CLOSED TREE
Attachment #9049535 - Attachment description: Bug 1533755 - Add SharedScriptData constructor. r?jandem → Bug 1533755 - Add SharedScriptData constructor. r?jwalden
Assignee

Comment 8

2 months ago

Mistake was qualifying DeletePolicy with unnecessary JS:: namespace which GCC complains about but Clang does not. Updated patches on phabricator will land once try comes back green.

Flags: needinfo?(tcampbell)
Assignee

Updated

2 months ago
Status: NEW → RESOLVED
Last Resolved: 2 months ago
Keywords: leave-open
Resolution: --- → FIXED
Assignee

Updated

2 months ago
Blocks: 1535138
You need to log in before you can comment on or make changes to this bug.