Skip to content

perf: index variable descriptors by a global ordinal - #2750

Open
triceo wants to merge 2 commits into
TimefoldAI:mainfrom
triceo:global-ordinal
Open

triceo wants to merge 2 commits into
TimefoldAI:mainfrom
triceo:global-ordinal

Conversation

@triceo

@triceo triceo commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator

Variable descriptors were only numbered within their entity, so hot lookups used two-level arrays or maps keyed by the variable meta-model, and per-entity indexing could collide under entity inheritance.

Each variable descriptor now gets a solution-wide ordinal at construction, from an offset its entity descriptor derives from the previously built one. Variable dispatch, value range statistics and the declarative graph lookups use flat arrays indexed by it; ordinal-based comparators use it too. This also fixes value counts of inherited variables in problem size statistics. Value counts per entity class now follow the order of the class's genuine variables.

Variable descriptors were only numbered within their entity, so hot lookups
used two-level arrays or maps keyed by the variable meta-model, and per-entity
indexing could collide under entity inheritance.

Each variable descriptor now gets a solution-wide ordinal at construction,
from an offset its entity descriptor derives from the previously built one.
Variable dispatch, value range statistics and the declarative graph lookups
use flat arrays indexed by it; ordinal-based comparators use it too.
This also fixes value counts of inherited variables in problem size statistics.
Value counts per entity class now follow the order of the class's genuine variables.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@triceo
triceo requested review from a team and a balanced review from Copilot October 14, 2026 05:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The implementation is consistent across descriptors, dispatch, graph lookup, statistics, and focused inheritance tests.

0 open findings

What changed in this PR

This PR adds solution-wide variable ordinals to prevent inheritance collisions and improve lookup performance.

Changes:

  • Replaces nested maps and arrays with flat ordinal-indexed arrays.
  • Corrects inherited-variable value statistics.
  • Adds inheritance and ordinal tests.
File Description
TestdataEntityRangeSolution.java Adds an inheritance statistics test solution.
TestdataEntityRangeChildEntity.java Adds the child test entity.
TestdataEntityRangeBaseEntity.java Adds an entity-provided value range.
VariableDescriptorGlobalOrdinalTest.java Verifies dense, unique global ordinals.
ProblemSizeStatisticsTest.java Tests inherited-variable value counts.
ValueRangeStatistics.java Uses ordinal-indexed value-count arrays.
ShadowVariablesAssert.java Orders variables by global ordinal.
VariableSupport.java Uses a flat dispatch array.
VariableDescriptor.java Adds the global ordinal.
DefaultShadowVariableSessionFactory.java Uses ordinals for graph lookups.
AbstractVariableReferenceGraph.java Replaces lookup maps with an indexed array.
SolutionDescriptor.java Exposes the variable descriptor count.
DefaultPlanningVariableMetaModel.java Compares variables by global ordinal.
EntityDescriptor.java Computes entity variable ordinal offsets.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The ordinal migration is internally consistent and has focused inheritance and statistics coverage.

0 open findings

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 14, 2026 12:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The global ordinal invariant is applied consistently and covered by inheritance and statistics tests.

0 open findings

🧠 Review effort: Balanced

This branch was successfully deployed

1 active deployment
internal — 9e64d356 Deployed Oct 14, 2026 by triceo via approval_required #5075
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants