ICU-23376 Minimize the number of states of the DFA

[eggrobin]

This halves the size of the line*.brk (from 73 424 bytes to 37 224 for line.brk, similarly for the others); it does nothing for char.brk or sent.brk, and not much for word.brk (23 120 to 22 672).

The C++ implementation is atrocious because of our lack of decent data structures, this was 30 lines of Python and would probably not be very much more in modern C++…

CC @aheninger, @robertbastian.

Checklist

• Required: Issue filed: ICU-23376
• Required: The PR title must be prefixed with a JIRA Issue number. Example: "ICU-NNNNN Fix xyz"
• Required: Each commit message must be prefixed with a JIRA Issue number. Example: "ICU-NNNNN Fix xyz"
• Issue accepted (done by Technical Committee after discussion)
• Tests included, if applicable
• API docs and/or User Guide docs changed or added, if applicable
• Approver: Feel free to merge on my behalf

[markusicu]

Going down to 199 states should let us use an 8-bit-value trie and 8-bit-value state table, right?
Does the builder select those automatically if possible?

[markusicu]

Java: future commit here or future PR?

[eggrobin]

Going down to 199 states should let us use an 8-bit-value trie and 8-bit-value state table, right?
Does the builder select those automatically if possible?

Looks like it:

icu/icu4c/source/common/rbbitblb.cpp

Lines 1362 to 1364 in c5946d7

	bool RBBITableBuilder::use8BitsForTable() const {
	return fDStates->size() <= kMaxStateFor8BitsTable;
	}

[markusicu]

This halves the size of the line*.brk (from 73 424 bytes to 37 224 for line.brk

🤯

[markusicu]

Going down to 199 states should let us use an 8-bit-value trie and 8-bit-value state table, right?
Does the builder select those automatically if possible?

Looks like it:

Yeah!

We might have a new problem then: Lack of code coverage for the 16-bit paths. Unless we have a unit test with strangely convoluted rules.

[eggrobin]

Java: future commit here or future PR?

I think either works, so depends on whether you review faster than I write the Java (you have an edge, I have half of Monday off). We probably can’t regenerate the checked-in .brks until I have done that because of round-trip tests.

[markusicu]

I am not trying to understand the logic.

[markusicu]

PS: Where I ask a question for why you choose something, and there is a good reason to do it that way, consider maybe adding a code comment.

[markusicu]

Looking at your local time, I think you are practicing for the UTC meeting...

[markusicu]

Sadly, CI checks are failing. I should bike home, and you should catch some sleep...

[eggrobin]

Looking at your local time, I think you are practicing for the UTC meeting...

Yeah, I am not sure what is going on here. This morning I woke up at seven in the morning. 24 h prior I was failing to sleep at seven in the morning.

[eggrobin]

Sadly, CI checks are failing

Yeah I think the unique_ptr is dragging in some silly symbols. Once we have a make_unique with a status we should not let that stop us, but for now I guess I will replace the remaining one with a LocalArray.

[eggrobin]

In any case, this needs TC approval, so it’s not getting merged tonight.

[eggrobin]

It looks like the fuzzer failure is just telling us that this minimization algorithm can be a bit slow on pathological state machines.
On the rules .+䶝.䶝.............;, it starts out with 7398 states and does nothing, which means that it needs to refine the partition from two parts to 7398. On my machine this takes 10 s in debug mode and 2 s in release mode. It does not seem implausible that it would exceed 25 s with ubsan on the CI runners.

[eggrobin]

@markusicu It looks like I successfully appeased the fuzzers. The windows-msys2-gcc-x86_64 failure seems unrelated, I see it on other PRs too.

[markusicu]

The windows-msys2-gcc-x86_64 failure seems unrelated, I see it on other PRs too.

Yeah, it fails on main too: https://github.com/unicode-org/icu/actions/runs/25382815834
This is very recent :-(

[markusicu]

The windows-msys2-gcc-x86_64 failure seems unrelated, I see it on other PRs too.

Yeah, it fails on main too: https://github.com/unicode-org/icu/actions/runs/25382815834 This is very recent :-(

Mihai has unblocked CI via PR #3970. Please rerun the failing test. If it still fails, try rebasing.

[jira-pull-request-webhook]

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot