1863467 - Can permanent atoms zone and table be stored as static data?

Status: NEW

(Core :: JavaScript Engine, task, P3)

Description

[Tooru Fujisawa [:arai]]

The WIP patch stack for bug 1850344 and bug 1848278 hits awsy regression (+50% Base Content JS), given all WebIDL atoms are going to be eagerly allocated.

Possible workaround is to move the entire permanent atom zone and related tables into static data, replacing of the source data (length, hash, content in WellKnownAtomInfo).

Plan:

1. make it possible to statically allocate the permanent atom zone and chunk
2. statically allocate JSAtom in the static atom zone
3. statically generate the atom table

For SpiderMonkey internal things, constexpr might work, but if we're to apply this also to atoms provided by embedding, python build script might be better.

Concern:

• JSAtom is 24-40 bytes, which is larger than the source data (length + hash + content), and this affects the file size more (there are ~1500 atoms internally, and ~8500 atoms in WebIDL)
• chunk data needs to be aligned (1MB). I'm not sure if this works in all situation, and wonder if it can consume more space by padding
• if we put string content in js::FatInlineAtom, it's hard to deduplicate the string with other code (WebIDL atom strings are used also by profiler labels: see bug 1850344 comment #10)
• having 2 places to generate zone data (static vs dynamic) could be a source of bug

Comment 1

[Jon Coppeard (:jonco)]

(In reply to Tooru Fujisawa [:arai] from comment #0)

The WIP patch stack for bug 1850344 and bug 1848278 hits awsy regression (+50% Base Content JS), given all WebIDL atoms are going to be eagerly allocated.

We should expect a memory regression from allocating this data up front. The plan would move this cost to the binary, but would it save any memory overall? Perhaps we should take the regression.

Comment 2

[Tooru Fujisawa [:arai]]

(discussed on chat)
apparently the memory win happens only for the source data of atomization (such as js::WellKnownAtomInfo array), which can be moved into the JSAtom itself.

we will have ~10000 permanent atoms for JS builtins and WebIDL atoms, that takes 160kB for hash+length+pointer and ~100kB for string content,
so ~260kB in total,
but the +50% Base Content JS regression is ~800kB.

https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=d1fb5fee2850ecd39aa7dba94457ff92ff294fd4&newProject=try&newRevision=9700e6cefb1d1aa7d2e84f8fe693066cc31f9b94&framework=4&page=1

so even if everything goes well, we'll still have 500kB+ regression.

if the amount of regression isn't good, I might have to revisit the entire plan, such as:

• reduce the target of this optimization
• do pre-atomization only for commonly-used atoms
• statically calculate hash+length, but don't do pre-atomization

Comment 3

[Tooru Fujisawa [:arai]]

(update from chat with mccr8 and jonco)

if we can make the static data pointer-less, the static data is shared between all processes, and it's beneficial for memory consumption, especially with fission.

Chunk contains some pointer fields, and if we're to make the chunk in static data pointer-less, we'll need some refactor,
or maybe use nullptr as special case.
next/prev can be nullptr as long as all atoms fit into single chunk.
storeBuffer and runtime may need special handling.

https://searchfox.org/mozilla-central/rev/02841791400cf7cf5760c0cfaf31f5d772624253/js/public/HeapAPI.h#99,102-103

struct TenuredChunkInfo {
...
  TenuredChunk* next = nullptr;
  TenuredChunk* prev = nullptr;

https://searchfox.org/mozilla-central/rev/02841791400cf7cf5760c0cfaf31f5d772624253/js/public/HeapAPI.h#77,92,95

class alignas(CellAlignBytes) ChunkBase {
...
  StoreBuffer* storeBuffer;
...
  JSRuntime* runtime;

Arena also has pointer fields, but next can be nullptr.
zone will need special handling.

https://searchfox.org/mozilla-central/rev/02841791400cf7cf5760c0cfaf31f5d772624253/js/src/gc/Heap.h#158,184,191

class alignas(ArenaSize) Arena {
...
  JS::Zone* zone;
...
  Arena* next;

Zone doesn't have to be static data, and it just need to point static arena/chunk.

Inline atoms doesn't contain pointer, but non-inline atoms contain pointer,
so this part needs some rework, or maybe use this static data only for inline atoms.

Also, the existence of pointer also applies to the existing static data (e.g. js::WellKnownAtomInfo), and we could revisit the data structure as well.