491850 - We want one code manager per AvmCore, not one per PoolObject.

Status: RESOLVED WONTFIX

(Tamarin Graveyard :: Baseline JIT (CodegenLIR), defect)

Description

[Edwin Smith]

One per pool object is too many small allocators, better to have a single larger one that owns all the compiled code for a VM instance.  Since we will be able to free individual methods, this will be okay.

Comment 1

[Edwin Smith]

Attachment: rename PageMgr to CodeManager, use just one per AvmCore

This also cleans up the code for setting the jitted code on the method (new method is MethodInfo.set_jit_code())

Comment 2

[Edwin Smith]

This change impacts unloading.  Before it, each PoolObject has a CodeAlloc, and ~CodeAlloc() gives pages back to GCHeap.  When PoolObjects are swept, either due to unloading or shutdown, code memory goes back to GCHeap, and it works fine as long as GCHeap outlives everything else.

With the above patch, ~CodeAlloc() doesn't run until AvmCore is swept.  ~PoolObject() and ~MethodInfo() have no logic for freeing code.  Furthermore, CodeMgr keeps a list of all jit'd methods, which pins MethodInfo and therefore PoolObject.  

CodeMgr.free_idle_methods() calls CodeMgr.sweep(), gives code back to CodeMgr, and de-referenced those idle MethodInfo's.  For testing purposes, free_idle_methods is called each time we start an incremental mark.  Since we wouldn't free any unloaded code until then anyway, this seems fine.

In production we don't plan to call free_idle_methods() until a low memory condition occurs.  Which means code won't finish unloading until we hit panic conditions.

Comment 3

[Edwin Smith]

Given our direction with background compilation, is this bug even desireable?  if not, i may close as wontfix.

- one per pool per bg compiler thread
- one per bg compiler thread (cross pools?)
- one per core?

Comment 4

[Simon Wilkinson]

FYI: the bg compilation implementation currently has one CodeMgr per pool per bg compiler thread. However, as the main thread runs the InvokeCompiler it also counts as a compiler thread and gets a CodeMgr per pool.

One CodeMgr per thread would work for me.

Comment 5

[Rick Reitmaier]

The one codeMgr per thread might pose an issue for unloading a PoolObject, unless as Ed mentions above we do 1 / pool / thread;  Or we tie a thread to a pool.

Or a simpler scheme would be to only allow 1 method from a pool to be compiled at a time.  This will enforce that only a single thread is holding a codeMgr from a given pool.

Comment 6

[Edwin Smith]

Updates:
CodeAlloc supports freeing individual methods with coalescing free blocks.
If anything, we'll probably want 1 CodeMgr per Toplevel or AbcEnv, but
thats out of scope.  Closing this bug as not-useful.