Skip to content

Codechange: cache seen names in the casefold guard, for performance - #452

Open
andythenorth wants to merge 1 commit into
OpenTTD:masterfrom
andythenorth:casefold-guard-perf
Open

andythenorth wants to merge 1 commit into
OpenTTD:masterfrom
andythenorth:casefold-guard-perf

Conversation

@andythenorth

Copy link
Copy Markdown
Contributor

#439 increased pre-processing time from ~4s to ~16s with Iron Horse nml.

The checking of similar names loops all of item names with a comparator. This will be some O(n) complexity that I am not skilled enough to figure out.

This patch puts a crude cache in of already seen names, using dict key lookup, which is usually fast.

With the patch applied, Iron Horse compile is ~4s.

However adding a cache usually means there are now 2 problems. I am unclear if anything else would need to fill or read this cache.

PeterN
PeterN previously approved these changes Oct 26, 2026

@LordAro LordAro left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure I follow - item_names is already a dict, why can't that be used, rather than adding a new container?

if self.name.value in global_constants.item_names:
   ...
else:
    casefolded = self.name.value.casefold()
    if casefolded in global_constants.item_names:
        ...
    else:
        ...

(could use := to allow an elif chain)

@PeterN
PeterN dismissed their stale review October 26, 2026 09:41

No idea what I'm doing

@PeterN PeterN left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Although item_names is already a dict, the names within it are not casefolded, so I guess a second dict of casefolded names is required

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.

3 participants