Closed Bug 1665797 Opened 5 years ago Closed 5 years ago

Shrink Prefable arrays by using a better ordering for specs.

Categories

(Core :: DOM: Bindings (WebIDL), defect)

defect

Tracking

()

RESOLVED FIXED
83 Branch
Tracking Status
firefox83 --- affected

People

(Reporter: n.nethercote, Assigned: n.nethercote)

Details

Attachments

(1 file, 1 obsolete file)

They currently use a sub-optimal ordering.

DOM bindings contain tables like this:

static const Prefable<const JSFunctionSpec> sMethods[] = {
  { nullptr, &sMethods_specs[0] },
  { &sMethods_disablers5, &sMethods_specs[5] },
  { nullptr, &sMethods_specs[8] },
  { &sMethods_disablers10, &sMethods_specs[10] },
  { nullptr, &sMethods_specs[13] },
  { &sMethods_disablers15, &sMethods_specs[15] },
  { nullptr, &sMethods_specs[18] },
  { &sMethods_disablers46, &sMethods_specs[46] },
  { &sMethods_disablers49, &sMethods_specs[49] },
  { nullptr, &sMethods_specs[51] },
  { &sMethods_disablers56, &sMethods_specs[56] },
  { nullptr, &sMethods_specs[59] },
  { &sMethods_disablers61, &sMethods_specs[61] },
  { nullptr, &sMethods_specs[64] },
  { &sMethods_disablers67, &sMethods_specs[67] },
  { &sMethods_disablers69, &sMethods_specs[69] },
  { &sMethods_disablers71, &sMethods_specs[71] },
  { nullptr, &sMethods_specs[75] },
  { nullptr, nullptr }
};

To save space, this table groups together all the adjacent specs that share a
disabler.

But there is repetition, because some non-adjacent specs have the same disabler
(or no disabler).

This commit sorts the specs so that all the specs with the same disablers are
adjacent, giving this:

static const Prefable<const JSFunctionSpec> sMethods[] = {
  { nullptr, &sMethods_specs[0] },
  { &sMethods_disablers47, &sMethods_specs[47] },
  { &sMethods_disablers50, &sMethods_specs[50] },
  { &sMethods_disablers52, &sMethods_specs[52] },
  { &sMethods_disablers54, &sMethods_specs[54] },
  { &sMethods_disablers61, &sMethods_specs[61] },
  { &sMethods_disablers64, &sMethods_specs[64] },
  { &sMethods_disablers66, &sMethods_specs[66] },
  { &sMethods_disablers69, &sMethods_specs[69] },
  { nullptr, nullptr }
};

As a result, the spec order in the generated C++ no longer matches the order
declared in .webidl files, but this doesn't seem like a big deal.

On Linux64 this reduces the binary size and the memory use per process by 20 KiB.

I had to draw this on paper to understand how all this stuff works. Might as
well save someone else (possibly me) the trouble in the future.

Depends on D90634

I have switched the order of the patches and landed the one with the comment.

If the change of ordering for the properties is a problem, that's ok, I can just abandon the change.

Pushed by nnethercote@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3c1b9452c0e6 Add a big comment about NativeProperties. r=mccr8 DONTBUILD
Pushed by nnethercote@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2ce16fca3ee4 Add a big comment about NativeProperties. r=mccr8 DONTBUILD
Flags: needinfo?(n.nethercote)
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 83 Branch
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #9176432 - Attachment is obsolete: true

Let's declare victory with the comment patch having landed.

Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: