Ken Domino
Ken Domino
OK, [the table for PredictionContext was an array](https://github.com/antlr/antlr-php-runtime/blob/ac71eb377c816c886b674a7ef805b54ca881d96c/src/Atn/ATNSimulator.php#L95), not implemented as a [Map](https://github.com/antlr/antlr4/blob/76fa05c21b12b96a6c12a0a82e611ed9d87d5af4/runtime/CSharp/src/Atn/PredictionContext.cs#L554), [newed here](https://github.com/antlr/antlr4/blob/76fa05c21b12b96a6c12a0a82e611ed9d87d5af4/runtime/CSharp/src/Atn/ATNSimulator.cs#L89). It was not working. I rewrote that to Map as in C#, and it now...
~~Not sure why, but debugging side-by-side of pdp7 grammar, input file s1.s, [first breakpoint here has 'altsets' containing just 2 items](https://github.com/antlr/antlr4/blob/2c75e642796718273fd215b03a310d4958319bad/runtime/CSharp/src/Atn/PredictionMode.cs#L874) in C#, ***31 items*** in [PHP](https://github.com/antlr/antlr-php-runtime/blob/ac71eb377c816c886b674a7ef805b54ca881d96c/src/Atn/PredictionMode.php#L581). It should not...
Although a minor issue, [this code in PHP does not have a cardinality check for alt sets](https://github.com/antlr/antlr-php-runtime/blob/ac71eb377c816c886b674a7ef805b54ca881d96c/src/Atn/PredictionMode.php#L586) as done [in C#](https://github.com/antlr/antlr4/blob/2c75e642796718273fd215b03a310d4958319bad/runtime/CSharp/src/Atn/PredictionMode.cs#L878). That would cut short the looping through the 31...
Bug, but not the problem https://github.com/antlr/antlr4/blob/76fa05c21b12b96a6c12a0a82e611ed9d87d5af4/runtime/CSharp/src/Atn/ATNConfigSet.cs#L280 (cached hash value should be reset to null, not -1, since everywhere else it's reset to null.)
Not that it makes much difference, but the hash code for an ATN state is the state number as in [Java](https://github.com/antlr/antlr4/blob/76fa05c21b12b96a6c12a0a82e611ed9d87d5af4/runtime/Java/src/org/antlr/v4/runtime/atn/ATNState.java#L131) and [CSharp](https://github.com/antlr/antlr4/blob/76fa05c21b12b96a6c12a0a82e611ed9d87d5af4/runtime/CSharp/src/Atn/ATNState.cs#L45), not the state type as in [PHP](https://github.com/antlr/antlr-php-runtime/blob/ac71eb377c816c886b674a7ef805b54ca881d96c/src/Atn/States/ATNState.php#L155)....
An absolutely humongous number of constructors are called for ATNConfig. It's easily 11x that for C# and Java. That's likely a symptom of the problem. Not sure why it's calling...
> > An absolutely humongous number of constructors are called for ATNConfig. It's easily 11x that for C# and Java. That's likely a symptom of the problem. Not sure why...
10% seems to be saved just by rewriting the hash function for ATNConfig. The code before the change looks like this: ``` public function hashCode(): int { return Hasher::hash( $this->state->stateNumber,...
> @kaby76 any updates on the PHP target fixes? I've been working on the grammars-v4 build because of various issues there. https://github.com/antlr/grammars-v4/issues/2991 https://github.com/antlr/grammars-v4/issues/2988 https://github.com/antlr/grammars-v4/issues/2883 I do know there were some...
Yes, it is a concern when things change in the source and the PR becomes out of date. I do plan to return to this in a week.