[RefC] Object Immortalization and Pre-Generation of Constants #3242
Conversation
* Shrink Value_Header to improve utilization of memory.
* When refCounter reaches its maximum, the object is immortalized to
avoid overflow. This immotalization is also used to represent
statically allocated stock objects.
* Prepare some commonly seen values and share it to improve memory usage.
* Added debug code to dump memory stats.
* Commonly seen values such as integers less than 100 are predefined and shared. * Constant String, Int64, Bits64 and Double values are allocated statically as indestructible and shared.
Refactor constantName function to return Core String type and handle unsupported types by throwing InternalError exception.
src/Compiler/RefC/RefC.idr
Outdated
| Ch x => pure "idris2_mkChar(\{escapeChar x})" | ||
| Str _ => orStagen | ||
| PrT t => pure $ cPrimType t | ||
| _ => pure "NULL" |
There was a problem hiding this comment.
this catch-all feels a bit brittle to me at first look at least
There was a problem hiding this comment.
In the last commit, I fixed this to improve coverage.
src/Compiler/RefC/RefC.idr
Outdated
| B64 x => pure "((Value*)&idris2_constant_Bits64_\{show x})" | ||
| Db x => pure "((Value*)&idris2_constant_Double_\{cCleanString $ show x})" | ||
| Str x => pure "((Value*)&idris2_constant_String_\{n})" | ||
| _ => throw $ InternalError "[refc] Unsupported type of constant." |
There was a problem hiding this comment.
this compile-time error feels like it could be fairly easily remediated by a new smaller data-type than Constant so that we aren't relying on the invariant that no new type of constant is stored in ConstDef without a new case being added here.
There was a problem hiding this comment.
I pushed the changes.
I hope I understand the problem correctly. Anyway, this eliminated the runtime error
| } else if (elem->header.refCounter != 1) { | ||
| --elem->header.refCounter; | ||
| return; | ||
| } else { | ||
| IDRIS2_INC_MEMSTAT(n_freed); |
There was a problem hiding this comment.
I notice that whereas elem->header.refCounter used to be decremented prior to freeing, now it is not; that seems theoretically ok to me, but I wanted to make sure it was intentional because I have not verified that nothing downstream of this code would double check the ref count.
There was a problem hiding this comment.
I decided not to write back the 0 value of the counter because I thought the overhead of updating the contents of a deleted object would be wasted.
The improvements are so small that they have little real effect.
Co-authored-by: Mathew Polzin <[email protected]>
|
@seagull-kamome @mattpolzin any chance we can get this closer to a resolution? |
|
I had some requests related to the Idris2 code that I’d like to see discussed if not acted upon but overall the LR felt close the last time I looked it over. |
|
I apologize for any inconvenience caused to you due to my poor time control. I just committed the latest code. I hope this clears up all doubts. |
|
No apology needed. None of us are making any implicit promises by volunteering our time including you. Thanks for following up. It’s an exceedingly busy time for me with a 3 day old baby but I’ll re-review this when I can ❤️. |
Description
This PR immortalizes objects whose reference counter has reached its maximum value and makes them excluded from GC. This immortalization is used to make the following improvements.
Statically generate values that often appear at runtime, such as empty strings and integers less than 100, and share them at runtime to reduce allocation costs. This heuristic may attract many opinions, but it is a good first-try choice.
On the other hand, there is no way to release immortal objects, so they leak. Dynamic immortalization rarely occurs, but it is a problem for programs that run for long periods of time, such as servers. A secondary GC is needed to solve this, but is not implemented in this PR.
Should this change go in the CHANGELOG?
implementation, I have updated
CHANGELOG_NEXT.md(and potentially alsoCONTRIBUTORS.md).