Opened 2 years ago

Last modified 10 months ago

#5979 new enhancement

Don't initialise null/boolean values in components.

Reported by: Freagarach Owned by:
Priority: Should Have Milestone: Backlog
Component: Simulation Keywords: simple
Cc: Patch: Phab:D3468

Description (last modified by Freagarach)

In many JS components there are a lot of variables created on init that are not strictly necessary.

E.g. in ResourceGatherer: this.lastCarriedType = undefined;.

There are more such examples of which some can have more effect on performance. It would be good to not initialise such values, but create them when needed and delete them when not needed anymore. This saves time on init and serialisation data during a game. (Refs. #3834.)

Also booleans are a good example of variables that need not to be initialised.

Some discussion was performed on Phab:rP25119 (text by smiley):

Maybe explicitly defining serializable properties might be cleaner. And if performance is the concern, setting a boolean is definitely faster than deleting properties and re initializing them again. There is also the issue of type optimizations potentially.

Potential performance impacts would depend on the JS engine, but I recall from some earlier benchmark that delete was kinda expensive since further lockups would result in searching the prototype chain. Also, it might change the object layout. Moreover, SM JIT does not play well with delete on arrays and hitting array holes was like hitting a wall. Given the underlying JS array implementation, I would not be surprised if the same behavior was found in objects as well.

I haven't redone those tests in current SM version. So, take that however you will.

Change History (13)

comment:1 by Freagarach, 2 years ago

Patch: Phab:D3468

comment:2 by Freagarach, 2 years ago

In r24993:

Do not initialise null values in cmpGarrisonHolder.

Saves some serialisation data (refs #3834).

Ticket: #5979
Differential revision: ​Phab:D3468
Comments by: @Angen, @Stan

comment:3 by Freagarach, 23 months ago

In 25030:

Give the Garrisonable component the power to set garrisoned on an entity with UnitAI, instead of leaving it to UnitAI.

Since the only way of garrisoning is using cmpGarrisonable.
r9498 / rP9498 introduced the this.isGarrisoned-flag, its meaning was discussed in Phab:D1403.

Refs. #5979, #6081
Differential revision: D2379
Comments by: @elexis, @Stan, @wraitii

comment:4 by Freagarach, 23 months ago

Description: modified (diff)
Summary: Don't initialise null values in components.Don't initialise null/boolean values in components.

comment:5 by Freagarach, 23 months ago

In 25064:

Clean up SpawnUnits in ProductionQueue.

Have an entity cache per item (refs. #6104) (which means it doesn't need to be created at init; refs. #5979).
Cache the PQ-position instead of querying it for every spawned entity.
One could use any arbitrary player for spawning entities now, increasing mod support.

Differential revision: D3669
Comment by: @wraitii

comment:6 by Freagarach, 23 months ago

In 25068:

Clean mirage internals.

Use a map that stores the functions instead of them being part of the Mirage component.
Means that a lot doesn't need to be inited now (refs. #5979).

Ticket: #5985
Differential revision: D3694
Comments by: @wraitii

comment:7 by Freagarach, 23 months ago

In 25079:

Clean mirage internals.

Redo r25068 / rP25068. Now! with not broken serialisation :D

Use a map that stores the functions, instead of them being part of the Mirage component.
Doesn't need to initialise the variables anymore (refs. #5979).

Saves the mirages in a global to allow (de)serialisation.
Only assign values on Init, not on Construction (to allow deserialisation).

Refs: #5985
Differential revision: D3702
Comments by: @wraitii

comment:8 by Freagarach, 23 months ago

In 25119:

A bit more cleanup in ProductionQueue.

  • Removed some useless comments.
  • Don't assume an item only has either a tech or a unit in ProgressTimeout.
  • Don't initialise boolean values (refs. #5979).

Differential revision: D3739
Comment by: @wraitii

comment:9 by Freagarach, 23 months ago

Description: modified (diff)

comment:10 by wraitii, 20 months ago

Milestone: Alpha 25Alpha 26

comment:11 by Silier, 18 months ago

Keywords: simple removed
severity: simple

comment:12 by Silier, 17 months ago

Keywords: simple added

comment:13 by Freagarach, 10 months ago

Milestone: Alpha 26Backlog
Note: See TracTickets for help on using tickets.