Skip to content

perf: remove subtraction from encoding - #25

Merged
alexharri merged 1 commit into
masterfrom
remove-subtraction
Jul 28, 2025
Merged

perf: remove subtraction from encoding#25
alexharri merged 1 commit into
masterfrom
remove-subtraction

Conversation

@alexharri

Copy link
Copy Markdown
Owner

What

Remove subtraction count from forms encoding:

# Before
2;ur,,i,ar

# After
ur,,i,ar

Why

It was pointed out to me by @vthorsteinsson that the forms encoding does not need to contain the subtraction count since it can be inferred from the nominative suffix.

Removing the subtraction count makes the library a bit smaller (0.26 kB, to be exact). Here is the full build output before/after:

# Before
beygla.js
        Minified: 12.67 kB
        Gzipped: 4.74 kB
beygla.esm.js
        Minified: 12.6 kB
        Gzipped: 4.72 kB
strict.js
        Minified: 44.14 kB
        Gzipped: 14.84 kB
strict.esm.js
        Minified: 44.05 kB
        Gzipped: 14.82 kB
addresses.js
        Minified: 16.08 kB
        Gzipped: 5.83 kB
addresses.esm.js
        Minified: 16.01 kB
        Gzipped: 5.81 kB

# After
beygla.js
        Minified: 11.19 kB
        Gzipped: 4.48 kB
beygla.esm.js
        Minified: 11.12 kB
        Gzipped: 4.46 kB
strict.js
        Minified: 42.66 kB
        Gzipped: 14.56 kB
strict.esm.js
        Minified: 42.57 kB
        Gzipped: 14.54 kB
addresses.js
        Minified: 14.46 kB
        Gzipped: 5.48 kB
addresses.esm.js
        Minified: 14.39 kB
        Gzipped: 5.46 kB

@alexharri
alexharri merged commit 7de8efb into master Jul 28, 2025
1 check passed
@alexharri
alexharri deleted the remove-subtraction branch July 28, 2025 17:23
@vthorsteinsson

Copy link
Copy Markdown

You can still do better: Either cut off the nominative ending from the word stem (i.e. store "Guðmund": "ur,,i,ar" instead of "Guðmundur" + the nominative suffix "ur"), or (slightly less efficient) keep the number but omit the nominative suffix, since you already know it: "Guðmundur": "2;d,i,ar" (store just the acc, dat, gen suffixes).

@alexharri

Copy link
Copy Markdown
Owner Author

Either cut off the nominative ending from the word stem (i.e. store "Guðmund": "ur,,i,ar" instead of "Guðmundur" + the nominative suffix "ur")

@vthorsteinsson the library does not actually encode the names themselves (except for the strict version, but it doesn't map names to declension patterns. It just includes the set of names to apply declension to), so I don't think this is applicable (unless I'm misunderstanding your point here).

or keep the number but omit the nominative suffix, since you already know it: "Guðmundur": "2;d,i,ar" (store just the acc, dat, gen suffixes).

This would probably make the bundle size smaller, but I have a slight correctness concern. There is a check to ensure that the declension can actually apply to the name (added in #13). The suffix match for a declension pattern can be (and often is) shorter than the nominative suffix (subtraction).

Say that looking up Maron were to match the declension son,,son,syni (this case is made up to illustrate the potential issue), then the current name.endsWith(nominativeSuffix) check would prevent that pattern of declension from being applied. If we were to encode that as 3,,son,syni, then we couldn't check for that case and that pattern of declension would be (incorrectly) applied.

I'm not convinced that this is a problem, but I'm not convinced that it's not a problem either. The problem with validating such concerns is that this could only be a problem for a name that we don't have declension data for 😄

@alexharri

Copy link
Copy Markdown
Owner Author

Actually, when thinking about this a bit more, the issue I raised could be resolved by not compressing the trie when it could cause the suffix match length to be shorter than the nominative suffix. That would allow us to remove the "can apply" check entirely and allow us to implement your suggestion

Requiring longer suffix matches would increase the size of the trie, which increases the bundle size, but that might be offset by the more compact declension pattern encoding. It'd be interested to see what the net bundle size change would be. I'll prototype this soon to check!

Btw, thanks for the suggestion @vthorsteinsson!

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