[I] Investigate Source Generators and PCRE.NET as Replacements for System.Text.Regex [lucenenet]

["NightOwl888 (via GitHub)" <gi...@apache.org>]

NightOwl888 opened a new issue, #1300:
URL: https://github.com/apache/lucenenet/issues/1300

   ### Is there an existing issue for this?
   
   - [x] I have searched the existing issues
   
   ### Task description
   
   Upon reviewing the code that uses `System.Text.Regex`, I noticed that there are many parts blocked from using `ReadOnlySpan<char>` to drive the `Regex` because of lack of support on .NET Framework and .NET Standard 2.0. Furthermore, I discovered that there are [.NET regular expression source generators](https://learn.microsoft.com/en-us/dotnet/standard/base-types/regular-expression-source-generators) that can be utilized to generate optimized code based on a regex string rather than using `System.Text.Regex` rather than simply swapping to the new `ReadOnlySpan<char>` APIs of `System.Text.Regex`.
   
   Being that Lucene.NET utilizes the `Regex` class and sometimes accepts it as a parameter in public APIs we should investigate this prior to the Lucene.NET 4.8.0 release because finding a solution will likely result in some breaking API changes.
   
   > It is worth pointing out that the `Pattern` class in Java has a different API than the `Regex` class in .NET, so much of the port is already a high-level reinterpretation of the Lucene code rather than a line-by-line port. So, there isn't much concern with staying aligned with Lucene in this area other than making it functionally correct.
   
   Some areas are using `Regex` to do simple tasks like split strings and it would be far better to use lower-level techniques (such as range-based, or ref struct enumerator-based splitting) to do those operations than to allocate temporary string arrays just for processing purposes.
   
   > .NET has a [`MemoryExtensions.Split()`](https://learn.microsoft.com/en-us/dotnet/api/system.memoryextensions.split?view=net-10.0) enumeration operation that returns a `Range` that can then be used on the original text to get a slice, but it is only supported on .NET Core. ICU4N has similar functionality with the [`SplitTokenizerEnumerator`](https://github.com/NightOwl888/ICU4N/blob/main/src/ICU4N/Support/Text/SplitTokenizerEnumerator.cs) that will work on all TFMs that we could borrow, and it supports both splitting and trimming at the same time.
   
   But there are other usages of `System.Text.Regex`, such as interpreting regular expressions from file or user input where we may consider using source generators and/or switching to regular expression APIs that accept `ReadOnlySpan<char>`, depending on which is the most sensible option.
   
   ### Legacy Regex Support for `ReadOnlySpan<char>`
   
   Unfortuantely, [Microsoft doesn't currently support regular expression source generators prior to .NET Core](https://github.com/dotnet/runtime/issues/104212). Furthermore, not even the `Regex` class has support for `ReadOnlySpan<char>` prior to .NET Core. So, our options are:
   
   1. Backport the source generator to .NET Standard 2.0/.NET Framework 4.6.2
   2. Use a 3rd party library, such as [PCRE.NET](https://www.nuget.org/packages/PCRE.NET) that has support for `ReadOnlySpan<char>` on .NET Standard 2.0 to bridge the support gap
   3. Conditionally compile and leave .NET Standard 2.0/.NET Framework 4.6.2 unoptimized
   4. Leave all of the `Regex` code unoptimized until the next Lucene upgrade
   
   Considering that the code structure could change significantly to take advantage of the optimized APIs on .NET Core, conditionally compiling most places where everywhere regular expressions are used seems like a high maintenance burden. It would be better to seek a solution that localizes conditional compilation to a single file that can be used to replace `System.Text.Regex` rather than scattering conditional compilation all over the solution.
   
   The task at hand is to find a way to:
   
   1. Favor modern regular expression development techniques on .NET Core
   2. Factor out trivial usages of regular expressions where lower-level code is justified, such as splitting strings
   3. Bridge the gap between legacy TFMs and .NET Core in a way that reduces or eliminates duplicate business logic, while still observing point 1
   4. Create a procedure to decide when to favor source generators versus when to migrate to modern regex APIs that support System.Memory and document it for current and future porting efforts
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] Investigate Source Generators and PCRE.NET as Replacements for System.Text.Regex [lucenenet]

["paulirwin (via GitHub)" <gi...@apache.org>]

paulirwin commented on issue #1300:
URL: https://github.com/apache/lucenenet/issues/1300#issuecomment-4565187568

   > 4. Leave all of the Regex code unoptimized until the next Lucene upgrade
   
   +1. We have enough other work to do, and much larger I/O-based performance issues to address first. I agree that there is a potential API breaking change concern here, so my vote would be to hold this for a future post-4.8 release so we can get this release done.
   
   (Note that I am going to begin holding myself to this same standard as well, and I'm actively working to remove items from the beta 18 release if they can be held for later.)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] Investigate Source Generators and PCRE.NET as Replacements for System.Text.Regex [lucenenet]

["paulirwin (via GitHub)" <gi...@apache.org>]

paulirwin commented on issue #1300:
URL: https://github.com/apache/lucenenet/issues/1300#issuecomment-4770560089

   The community has not chimed in on this in over a month. Given no objections, I'm going to move this out of scope and into Future, as I don't think this is necessary for our 4.x release.
   
   (Aside: I personally hope a future post-4.x release can finally drop older TFM support so that this is unnecessary anyways. But that's a longer, separate discussion and doesn't need to be debated here.)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] Investigate Source Generators and PCRE.NET as Replacements for System.Text.Regex [lucenenet]

["paulirwin (via GitHub)" <gi...@apache.org>]

paulirwin commented on issue #1300:
URL: https://github.com/apache/lucenenet/issues/1300#issuecomment-4565216206

   Also, I would not assign High priority to this item. I think the API-break concern means that it would have to be now (beta 18) or later (future, post-4.8), but its target milestone is orthogonal to its priority. In my opinion, this would be low priority, or medium/normal at most, without demonstrated real-world performance benchmark concerns.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org