1432340 - Introduce vm/FreeOp.h to contain that class, extricating it from the far-more-ponderous vm/Runtime.h for those users needing *only* js::FreeOp and nothing else

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jeff Walden [:Waldo]]

Moving this out this way, means subsequent split-up Intl files can include just this file and avoid picking up vm/Runtime.h.

I used vm/FreeOp.h, but as FreeOp is mostly involved in finalizing and sweeping, at a quick skim of FreeOp uses, maybe it should be gc/FreeOp.h.  Feel free to ask for that move if desired.

Comment 1

[Jeff Walden [:Waldo]]

Attachment: Patch

Comment 2

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8944586 [details] [diff] [review]
Patch

Review of attachment 8944586 [details] [diff] [review]:
-----------------------------------------------------------------

Looking through the output of

    perl -lne '$f = $1 if /^([^\s{#].*)/; print $f if /fop->/' **/*.cpp

and piping it through

    perl -lne 'print $1 if /(\w+)\(/' | sort | uniq -c

it looks like the only non-obviously finalization functions that use FreeOp are

queueForBackgroundSweep
queueForForegroundSweep
sweep
sweepAll
sweepZoneGroups

decrementStepModeCount
FinishDiscardBaselineScript
FinishInvalidation
FreeJobQueueHandling
Invalidate
recompile
ReleaseAllJITCode
releaseData
ReleaseScriptCounts
setNewStepMode
unlinkDependentWasmImports
unlinkFromRuntime

So there's a little bit of stuff there that might not be directly sweep or finalization related, but I guess it feels GC-centric enough that I'm going to vote for gc/FreeOp.h. Even though this is not GC memory so it doesn't relate to gc/Allocator.h at all. But it does correspond to gc/Memory.h, which is all about the interface between the GC and the core memory allocation routines, and similarly this seems to be almost entirely about the interface between the GC and malloc/free.

It's still a little unsatisfying, but I'm swayed by the argument that if someone is looking for how to allocate and free some (non-GC) heap memory from the spidermonkey VM, I'd rather they not stumble across vm/FreeOp.h and get confused by it.

Sorry for the churn.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/60e1a827ac56
Introduce gc/FreeOp.h to contain that class, extricating it from the far-more-ponderous vm/Runtime.h for those users needing *only* js::FreeOp and nothing else.  r=sfink

Comment 4

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/60e1a827ac56