Don't create Machine while holding this' monitor. #85

sjamesr · 2019-02-22T17:39:05Z

This change attempts to reduce monitor contention.

@sjamesr

… & more * This PR ports [re2j](https://github.com/google/re2j/) PR #[85](google/re2j#85) "Don't create Machine while holding this' monitor", dated 2019-02-22, to Scala Native. My thanks and appreciation to the re2j developer @sjamesr for the re2j PR. * Since ScalaNative is currently single threaded, this PR may seem to be not worth the overhead. At the very least, it keeps the scalanative.regex/re2s implementation close to the re2j parent. In the longer run, I do believe that someday, Real Soon Now, a Big Switch will be thrown and not having synchronization issues that could & should have been avoided sprinkled all over the code base will be seen to be a Good Thing. * The port looks slightly different than one would expect from the normal java-to-scala differences. The essence of the re2j PR is maintained but there is a difference in implementation. ScalaNative does not yet implement the ArrayDeque introduced by re2j in its PR. This port retains the ArrayList implementation used by re2j before the PR and by re2s/sn_regex. The ArrayList has worked well for sn_regex. I studied ArrayDeque and it seemed to offer no obvious performance advantages, given that RE2.get() is implemented to remove the last element of the list. * ScalaNative Issue scala-native#1091 describes a bug where monitor methods are not generated when `synchronized` is used with `def`. It reports that the code generated for interior synchronized blocks is correct. There are three uses of `synchronized` in the scalanative.regex package. + This PR naturally changed RE2.get(). + RE2.reset() and RE2.put() were changed and the reason commented. * No unit-tests were added. The re2j PR had no changes associated with testing. Focus testing the behavior on a single threaded ScalaNative would be hard. While it can not be shown that this PR introduced the positive change it describes, it can be shown, to a reasonable degree of certitude, that it did not introduce obvious negative changes, a.k.a. bugs: `test-all` in release-fast mode succeeds with no new performance issues. Documentation: * The standard changelog entry is requested. Testing: * Built and tested ("test-all") in release-fast mode using sbt 1.2.8 on X86_64 only . All tests pass.

@sjamesr

* This PR ports [re2j](https://github.com/google/re2j/) PR #[79](google/re2j#85) "Add Regexp.hashCode and tests.", dated 2018-10-23, to Scala Native. My thanks and appreciation to the re2j developer @sjamesr for the re2j PR. * Regexp#equals existed in re2j was originally ported to SN but Regexp#hashCode did not. SN refused to compile with this mismatch, so the former was dropped from the port. Now that the pair is available in re2j, both routines has been ported & implemented. * The original scalanative.regex code used the default reference equality in Regexp. equals(). Changing Regexp#equals to use content equality necessitated a change in Simplify.scala to use reference equality in two critical places. * The original re2j code supported only the Perl syntax for named capture groups "(?P<foo?)". Java supports only a slightly different syntax "(?<foo?)". The original port to scalanative changed changed the code to support only the Java syntax. This PR changes Parser.scala to support both idioms. This allows two re2j test cases for equals() and hashCode() to execute and pass. The change was documented, see below. Two java.util.regex Suites were edited to allow the now supported Perl syntax. * The unit-test Suite RegexpHashcodeEqualsSuite.scala was added. Documentation: * The standard changelog entry is requested. * A section was added to `docs/lib/javalib.rst` to describe some RE2 Perl expressions now available in `java.util.regex`. These are not strictly Java 8 conformant but are likely to be useful to developers porting Perl regexen. Both the Java and Perl regular expressions for named capture groups are available. Testing: * Built and tested ("test-all") in release-fast mode using sbt 1.2.8 on X86_64 only . All tests pass. * A manual trial `sphinx-build -b html ` of javalib.rst was done and the result inspected in a browser.

@sjamesr

* This PR ports [re2j](https://github.com/google/re2j/) PR #[79](google/re2j#85) "Add Regexp.hashCode and tests.", dated 2018-10-23, to Scala Native. My thanks and appreciation to the re2j developer @sjamesr for the re2j PR. * Regexp#equals existed in re2j was originally ported to SN but Regexp#hashCode did not. SN refused to compile with this mismatch, so the former was dropped from the port. Now that the pair is available in re2j, both routines has been ported & implemented. * The original scalanative.regex code used the default reference equality in Regexp. equals(). Changing Regexp#equals to use content equality necessitated a change in Simplify.scala to use reference equality in two critical places. * The original re2j code supported only the Perl syntax for named capture groups "(?P<foo?)". Java supports only a slightly different syntax "(?<foo?)". The original port to scalanative changed changed the code to support only the Java syntax. This PR changes Parser.scala to support both idioms. This allows two re2j test cases for equals() and hashCode() to execute and pass. The change was documented, see below. Two java.util.regex Suites were edited to allow the now supported Perl syntax. * The unit-test Suite RegexpHashcodeEqualsSuite.scala was added. Documentation: * The standard changelog entry is requested. * A section was added to `docs/lib/javalib.rst` to describe some RE2 Perl expressions now available in `java.util.regex`. These are not strictly Java 8 conformant but are likely to be useful to developers porting Perl regexen. Both the Java and Perl regular expressions for named capture groups are available. Testing: * Built and tested ("test-all") in release-fast mode using sbt 1.2.8 on X86_64 only . All tests pass. * A manual trial `sphinx-build -b html ` of javalib.rst was done and the result inspected in a browser.

@sjamesr

… & more * This PR ports [re2j](https://github.com/google/re2j/) PR #[85](google/re2j#85) "Don't create Machine while holding this' monitor", dated 2019-02-22, to Scala Native. My thanks and appreciation to the re2j developer @sjamesr for the re2j PR. * Since ScalaNative is currently single threaded, this PR may seem to be not worth the overhead. At the very least, it keeps the scalanative.regex/re2s implementation close to the re2j parent. In the longer run, I do believe that someday, Real Soon Now, a Big Switch will be thrown and not having synchronization issues that could & should have been avoided sprinkled all over the code base will be seen to be a Good Thing. * The port looks slightly different than one would expect from the normal java-to-scala differences. The essence of the re2j PR is maintained but there is a difference in implementation. ScalaNative does not yet implement the ArrayDeque introduced by re2j in its PR. This port retains the ArrayList implementation used by re2j before the PR and by re2s/sn_regex. The ArrayList has worked well for sn_regex. I studied ArrayDeque and it seemed to offer no obvious performance advantages, given that RE2.get() is implemented to remove the last element of the list. * ScalaNative Issue scala-native#1091 describes a bug where monitor methods are not generated when `synchronized` is used with `def`. It reports that the code generated for interior synchronized blocks is correct. There are three uses of `synchronized` in the scalanative.regex package. + This PR naturally changed RE2.get(). + RE2.reset() and RE2.put() were changed and the reason commented. * No unit-tests were added. The re2j PR had no changes associated with testing. Focus testing the behavior on a single threaded ScalaNative would be hard. While it can not be shown that this PR introduced the positive change it describes, it can be shown, to a reasonable degree of certitude, that it did not introduce obvious negative changes, a.k.a. bugs: `test-all` in release-fast mode succeeds with no new performance issues. Documentation: * The standard changelog entry is requested. Testing: * Built and tested ("test-all") in release-fast mode using sbt 1.2.8 on X86_64 only . All tests pass.

@sjamesr

… & more * This PR ports [re2j](https://github.com/google/re2j/) PR #[85](google/re2j#85) "Don't create Machine while holding this' monitor", dated 2019-02-22, to Scala Native. My thanks and appreciation to the re2j developer @sjamesr for the re2j PR. * Since ScalaNative is currently single threaded, this PR may seem to be not worth the overhead. At the very least, it keeps the scalanative.regex/re2s implementation close to the re2j parent. In the longer run, I do believe that someday, Real Soon Now, a Big Switch will be thrown and not having synchronization issues that could & should have been avoided sprinkled all over the code base will be seen to be a Good Thing. * The port looks slightly different than one would expect from the normal java-to-scala differences. The essence of the re2j PR is maintained but there is a difference in implementation. ScalaNative does not yet implement the ArrayDeque introduced by re2j in its PR. This port retains the ArrayList implementation used by re2j before the PR and by re2s/sn_regex. The ArrayList has worked well for sn_regex. I studied ArrayDeque and it seemed to offer no obvious performance advantages, given that RE2.get() is implemented to remove the last element of the list. * ScalaNative Issue scala-native#1091 describes a bug where monitor methods are not generated when `synchronized` is used with `def`. It reports that the code generated for interior synchronized blocks is correct. There are three uses of `synchronized` in the scalanative.regex package. + This PR naturally changed RE2.get(). + RE2.reset() and RE2.put() were changed and the reason commented. * No unit-tests were added. The re2j PR had no changes associated with testing. Focus testing the behavior on a single threaded ScalaNative would be hard. While it can not be shown that this PR introduced the positive change it describes, it can be shown, to a reasonable degree of certitude, that it did not introduce obvious negative changes, a.k.a. bugs: `test-all` in release-fast mode succeeds with no new performance issues. Documentation: * The standard changelog entry is requested. Testing: * Built and tested ("test-all") in release-fast mode using sbt 1.2.8 on X86_64 only . All tests pass.

@sjamesr

… & more * This PR ports [re2j](https://github.com/google/re2j/) PR #[85](google/re2j#85) "Don't create Machine while holding this' monitor", dated 2019-02-22, to Scala Native. My thanks and appreciation to the re2j developer @sjamesr for the re2j PR. * Since ScalaNative is currently single threaded, this PR may seem to be not worth the overhead. At the very least, it keeps the scalanative.regex/re2s implementation close to the re2j parent. In the longer run, I do believe that someday, Real Soon Now, a Big Switch will be thrown and not having synchronization issues that could & should have been avoided sprinkled all over the code base will be seen to be a Good Thing. * The port looks slightly different than one would expect from the normal java-to-scala differences. The essence of the re2j PR is maintained but there is a difference in implementation. ScalaNative does not yet implement the ArrayDeque introduced by re2j in its PR. This port retains the ArrayList implementation used by re2j before the PR and by re2s/sn_regex. The ArrayList has worked well for sn_regex. I studied ArrayDeque and it seemed to offer no obvious performance advantages, given that RE2.get() is implemented to remove the last element of the list. * ScalaNative Issue scala-native#1091 describes a bug where monitor methods are not generated when `synchronized` is used with `def`. It reports that the code generated for interior synchronized blocks is correct. There are three uses of `synchronized` in the scalanative.regex package. + This PR naturally changed RE2.get(). + RE2.reset() and RE2.put() were changed and the reason commented. * No unit-tests were added. The re2j PR had no changes associated with testing. Focus testing the behavior on a single threaded ScalaNative would be hard. While it can not be shown that this PR introduced the positive change it describes, it can be shown, to a reasonable degree of certitude, that it did not introduce obvious negative changes, a.k.a. bugs: `test-all` in release-fast mode succeeds with no new performance issues. Documentation: * The standard changelog entry is requested. Testing: * Built and tested ("test-all") in release-fast mode using sbt 1.2.8 on X86_64 only . All tests pass.

@sjamesr

… & more * This PR ports [re2j](https://github.com/google/re2j/) PR #[85](google/re2j#85) "Don't create Machine while holding this' monitor", dated 2019-02-22, to Scala Native. My thanks and appreciation to the re2j developer @sjamesr for the re2j PR. * Since ScalaNative is currently single threaded, this PR may seem to be not worth the overhead. At the very least, it keeps the scalanative.regex/re2s implementation close to the re2j parent. In the longer run, I do believe that someday, Real Soon Now, a Big Switch will be thrown and not having synchronization issues that could & should have been avoided sprinkled all over the code base will be seen to be a Good Thing. * The port looks slightly different than one would expect from the normal java-to-scala differences. The essence of the re2j PR is maintained but there is a difference in implementation. ScalaNative does not yet implement the ArrayDeque introduced by re2j in its PR. This port retains the ArrayList implementation used by re2j before the PR and by re2s/sn_regex. The ArrayList has worked well for sn_regex. I studied ArrayDeque and it seemed to offer no obvious performance advantages, given that RE2.get() is implemented to remove the last element of the list. * ScalaNative Issue scala-native#1091 describes a bug where monitor methods are not generated when `synchronized` is used with `def`. It reports that the code generated for interior synchronized blocks is correct. There are three uses of `synchronized` in the scalanative.regex package. + This PR naturally changed RE2.get(). + RE2.reset() and RE2.put() were changed and the reason commented. * No unit-tests were added. The re2j PR had no changes associated with testing. Focus testing the behavior on a single threaded ScalaNative would be hard. While it can not be shown that this PR introduced the positive change it describes, it can be shown, to a reasonable degree of certitude, that it did not introduce obvious negative changes, a.k.a. bugs: `test-all` in release-fast mode succeeds with no new performance issues. Documentation: * The standard changelog entry is requested. Testing: * Built and tested ("test-all") in release-fast mode using sbt 1.2.8 on X86_64 only . All tests pass.

@sjamesr

… & more * This PR ports [re2j](https://github.com/google/re2j/) PR #[85](google/re2j#85) "Don't create Machine while holding this' monitor", dated 2019-02-22, to Scala Native. My thanks and appreciation to the re2j developer @sjamesr for the re2j PR. * Since ScalaNative is currently single threaded, this PR may seem to be not worth the overhead. At the very least, it keeps the scalanative.regex/re2s implementation close to the re2j parent. In the longer run, I do believe that someday, Real Soon Now, a Big Switch will be thrown and not having synchronization issues that could & should have been avoided sprinkled all over the code base will be seen to be a Good Thing. * I do not understand why the upstream re2j changed the data structure from ArrayList to ArrayDeque, then uses it as a Queue. Seems like a slight pessimization, if ArrayDeque is implemented in terms of ArrayList, as it now is in Scala Native. It seemed kinder to future devos, including myself in three months, to follow the re2j change that to maintain lots of comments explaining why re2s/sn.regex and re2j differed. This is especially important because of rounds of re2j PRs I hope to port in the near future. * No unit-tests were added. The re2j PR had no changes associated with testing. Focus testing the behavior on a single threaded ScalaNative would be hard. While it can not be shown that this PR introduced the positive change it describes, it can be shown, to a reasonable degree of certitude, that it did not introduce obvious negative changes, a.k.a. bugs: `test-all` in release-fast mode succeeds with no new performance issues. Documentation: * The standard changelog entry is requested. Testing: * Built and tested ("test-all") in release-fast mode using sbt 1.2.8 on X86_64 only . All tests pass.

@sjamesr

… & more * This PR ports [re2j](https://github.com/google/re2j/) PR #[85](google/re2j#85) "Don't create Machine while holding this' monitor", dated 2019-02-22, to Scala Native. My thanks and appreciation to the re2j developer @sjamesr for the re2j PR. * Since ScalaNative is currently single threaded, this PR may seem to be not worth the overhead. At the very least, it keeps the scalanative.regex/re2s implementation close to the re2j parent. In the longer run, I do believe that someday, Real Soon Now, a Big Switch will be thrown and not having synchronization issues that could & should have been avoided sprinkled all over the code base will be seen to be a Good Thing. * I do not understand why the upstream re2j changed the data structure from ArrayList to ArrayDeque, then uses it as a Queue. Seems like a slight pessimization, if ArrayDeque is implemented in terms of ArrayList, as it now is in Scala Native. It seemed kinder to future devos, including myself in three months, to follow the re2j change that to maintain lots of comments explaining why re2s/sn.regex and re2j differed. This is especially important because of rounds of re2j PRs I hope to port in the near future. * No unit-tests were added. The re2j PR had no changes associated with testing. Focus testing the behavior on a single threaded ScalaNative would be hard. While it can not be shown that this PR introduced the positive change it describes, it can be shown, to a reasonable degree of certitude, that it did not introduce obvious negative changes, a.k.a. bugs: `test-all` in release-fast mode succeeds with no new performance issues. Documentation: * The standard changelog entry is requested. Testing: * Built and tested ("test-all") in release-fast mode using sbt 1.2.8 on X86_64 only . All tests pass.

@sjamesr

… & more * This PR ports [re2j](https://github.com/google/re2j/) PR #[85](google/re2j#85) "Don't create Machine while holding this' monitor", dated 2019-02-22, to Scala Native. My thanks and appreciation to the re2j developer @sjamesr for the re2j PR. * Since ScalaNative is currently single threaded, this PR may seem to be not worth the overhead. At the very least, it keeps the scalanative.regex/re2s implementation close to the re2j parent. In the longer run, I do believe that someday, Real Soon Now, a Big Switch will be thrown and not having synchronization issues that could & should have been avoided sprinkled all over the code base will be seen to be a Good Thing. * I do not understand why the upstream re2j changed the data structure from ArrayList to ArrayDeque, then uses it as a Queue. Seems like a slight pessimization, if ArrayDeque is implemented in terms of ArrayList, as it now is in Scala Native. It seemed kinder to future devos, including myself in three months, to follow the re2j change that to maintain lots of comments explaining why re2s/sn.regex and re2j differed. This is especially important because of rounds of re2j PRs I hope to port in the near future. * No unit-tests were added. The re2j PR had no changes associated with testing. Focus testing the behavior on a single threaded ScalaNative would be hard. While it can not be shown that this PR introduced the positive change it describes, it can be shown, to a reasonable degree of certitude, that it did not introduce obvious negative changes, a.k.a. bugs: `test-all` in release-fast mode succeeds with no new performance issues. Documentation: * The standard changelog entry is requested. Testing: * Built and tested ("test-all") in release-fast mode using sbt 1.2.8 on X86_64 only . All tests pass.

@sjamesr

… & more * This PR ports [re2j](https://github.com/google/re2j/) PR #[85](google/re2j#85) "Don't create Machine while holding this' monitor", dated 2019-02-22, to Scala Native. My thanks and appreciation to the re2j developer @sjamesr for the re2j PR. * Since ScalaNative is currently single threaded, this PR may seem to be not worth the overhead. At the very least, it keeps the scalanative.regex/re2s implementation close to the re2j parent. In the longer run, I do believe that someday, Real Soon Now, a Big Switch will be thrown and not having synchronization issues that could & should have been avoided sprinkled all over the code base will be seen to be a Good Thing. * I do not understand why the upstream re2j changed the data structure from ArrayList to ArrayDeque, then uses it as a Queue. Seems like a slight pessimization, if ArrayDeque is implemented in terms of ArrayList, as it now is in Scala Native. It seemed kinder to future devos, including myself in three months, to follow the re2j change that to maintain lots of comments explaining why re2s/sn.regex and re2j differed. This is especially important because of rounds of re2j PRs I hope to port in the near future. * No unit-tests were added. The re2j PR had no changes associated with testing. Focus testing the behavior on a single threaded ScalaNative would be hard. While it can not be shown that this PR introduced the positive change it describes, it can be shown, to a reasonable degree of certitude, that it did not introduce obvious negative changes, a.k.a. bugs: `test-all` in release-fast mode succeeds with no new performance issues. Documentation: * The standard changelog entry is requested. Testing: * Built and tested ("test-all") in release-fast mode using sbt 1.2.8 on X86_64 only . All tests pass.

@sjamesr

* Detect an invalid Unicode class specification. * This PR ports [re2j](https://github.com/google/re2j/) PR #[74](https://github.com/google/re2j/) "Detect invalid Unicode class specification", dated 2018-10-23, to Scala Native. Prior to this PR, compiling the pattern "\\p" would lead to an unexpected java.lang.StringIndexOutOfBoundsException. As of this PR the PatternSyntaxException used by the JVM is thrown. The text of the exception is slightly different. JVM reports the unknown property name, or {?}. Doing that in SN regex is harder that time allowed. * A new ERR_UNKNOWN_CHARACTER_PROPERTY_NAME message was added and and the initial word of several messages converted to Titlecase. Messages on the JVM start with an initial capital letter. Message texts from SN regex (née re2s) still do not match JVM exactly but they are moving closer. * The message text for the invalid specification "\\p{" was also changed. * Fix #1673: regex ANY_CHARACTER "\\n|." ANY_CHARACTER idiom now works * This PR was motivated by Issue #1673 " regex RE2 wrongly ignores newline in OR groups". The defect was a botched rewrite of the Parser.scala method matchRune() in an attempt to remove multiple `return` statements. The issue is now fixed for the two patterns which would evoke the presenting problem. The patterns "\\n|." and ".|\\n" are both used to indicate a match to ANY_CHARACTER, including newline. Because regex is defined to have a preference for the left term, they execute slightly different code paths. * Two unit-test cases were added to ParserSuite.scala. Each tests one of the two patterns which evoked Issue #1673. * Details for future maintainers. + The FLAGS argument to testParseDump() in NOMATCHNL_TESTS as 0. That is, all flags are clear. This mean MATCH_NL is clear, which matches Java 8 DOTALL being clear: dot does not match newline. Most of rest of Suite uses FLAGS which match PERL: `TEST_FLAGS = MATCH_NL | PERL_X | UNICODE_GROUPS` This sets dot to match newline and does not evoke Issue #1673. + Before this PR, Parser.scala with MATCH_NL clear compiled uses of the patterns to "dnl{}"; that is, ROP.ANY_CHAR_NOT_NL. As of this PR, the patterns get compiled to the correct and expected "dnl{}", ROP.ANY_CHAR. See NONMATCHNL_TESTS . Documentation: * The standard changelog entry is requested. * nativelib regex RE2.Machine mutual exclusion improvements, re2j PR 85 & more * This PR ports [re2j](https://github.com/google/re2j/) PR #[85](google/re2j#85) "Don't create Machine while holding this' monitor", dated 2019-02-22, to Scala Native. My thanks and appreciation to the re2j developer @sjamesr for the re2j PR. * Since ScalaNative is currently single threaded, this PR may seem to be not worth the overhead. At the very least, it keeps the scalanative.regex/re2s implementation close to the re2j parent. In the longer run, I do believe that someday, Real Soon Now, a Big Switch will be thrown and not having synchronization issues that could & should have been avoided sprinkled all over the code base will be seen to be a Good Thing. * I do not understand why the upstream re2j changed the data structure from ArrayList to ArrayDeque, then uses it as a Queue. Seems like a slight pessimization, if ArrayDeque is implemented in terms of ArrayList, as it now is in Scala Native. It seemed kinder to future devos, including myself in three months, to follow the re2j change that to maintain lots of comments explaining why re2s/sn.regex and re2j differed. This is especially important because of rounds of re2j PRs I hope to port in the near future. * No unit-tests were added. The re2j PR had no changes associated with testing. Focus testing the behavior on a single threaded ScalaNative would be hard. While it can not be shown that this PR introduced the positive change it describes, it can be shown, to a reasonable degree of certitude, that it did not introduce obvious negative changes, a.k.a. bugs: `test-all` in release-fast mode succeeds with no new performance issues. Co-authored-by: Lee Tibbert <lee.tibbert@gmail.com> Co-authored-by: Lee Tibbert <Lee Tibbert>

@sjamesr

* This PR ports [re2j](https://github.com/google/re2j/) PR #[79](google/re2j#85) "Add Regexp.hashCode and tests.", dated 2018-10-23, to Scala Native. My thanks and appreciation to the re2j developer @sjamesr for the re2j PR. * Regexp#equals existed in re2j was originally ported to SN but Regexp#hashCode did not. SN refused to compile with this mismatch, so the former was dropped from the port. Now that the pair is available in re2j, both routines has been ported & implemented. * The original scalanative.regex code used the default reference equality in Regexp. equals(). Changing Regexp#equals to use content equality necessitated a change in Simplify.scala to use reference equality in two critical places. * The original re2j code supported only the Perl syntax for named capture groups "(?P<foo?)". Java supports only a slightly different syntax "(?<foo?)". The original port to scalanative changed changed the code to support only the Java syntax. This PR changes Parser.scala to support both idioms. This allows two re2j test cases for equals() and hashCode() to execute and pass. The change was documented, see below. Two java.util.regex Suites were edited to allow the now supported Perl syntax. * The unit-test Suite RegexpHashcodeEqualsTest.scala was added. Documentation: * The standard changelog entry is requested. * A section was added to `docs/lib/javalib.rst` to describe some RE2 Perl expressions now available in `java.util.regex`. These are not strictly Java 8 conformant but are likely to be useful to developers porting Perl regexen. Both the Java and Perl regular expressions for named capture groups are available. Testing: * Built and tested ("test-all") in release-fast mode using sbt 1.2.8 on X86_64 only . All tests pass. * A manual trial `sphinx-build -b html ` of javalib.rst was done and the result inspected in a browser.

@sjamesr

* This PR ports [re2j](https://github.com/google/re2j/) PR #[79](google/re2j#85) "Add Regexp.hashCode and tests.", dated 2018-10-23, to Scala Native. My thanks and appreciation to the re2j developer @sjamesr for the re2j PR. * Regexp#equals existed in re2j was originally ported to SN but Regexp#hashCode did not. SN refused to compile with this mismatch, so the former was dropped from the port. Now that the pair is available in re2j, both routines has been ported & implemented. * The original scalanative.regex code used the default reference equality in Regexp. equals(). Changing Regexp#equals to use content equality necessitated a change in Simplify.scala to use reference equality in two critical places. * The original re2j code supported only the Perl syntax for named capture groups "(?P<foo?)". Java supports only a slightly different syntax "(?<foo?)". The original port to scalanative changed changed the code to support only the Java syntax. This PR changes Parser.scala to support both idioms. This allows two re2j test cases for equals() and hashCode() to execute and pass. The change was documented, see below. Two java.util.regex Suites were edited to allow the now supported Perl syntax. * The unit-test Suite RegexpHashcodeEqualsTest.scala was added. Documentation: * The standard changelog entry is requested. * A section was added to `docs/lib/javalib.rst` to describe some RE2 Perl expressions now available in `java.util.regex`. These are not strictly Java 8 conformant but are likely to be useful to developers porting Perl regexen. Both the Java and Perl regular expressions for named capture groups are available. Testing: * Built and tested ("test-all") in release-fast mode using sbt 1.2.8 on X86_64 only . All tests pass. * A manual trial `sphinx-build -b html ` of javalib.rst was done and the result inspected in a browser.

@sjamesr

* Port/implement s.n.regex.Regexp hashCode(), equals(), & tests. * This PR ports [re2j](https://github.com/google/re2j/) PR #[79](google/re2j#85) "Add Regexp.hashCode and tests.", dated 2018-10-23, to Scala Native. My thanks and appreciation to the re2j developer @sjamesr for the re2j PR. * Regexp#equals existed in re2j was originally ported to SN but Regexp#hashCode did not. SN refused to compile with this mismatch, so the former was dropped from the port. Now that the pair is available in re2j, both routines has been ported & implemented. * The original scalanative.regex code used the default reference equality in Regexp. equals(). Changing Regexp#equals to use content equality necessitated a change in Simplify.scala to use reference equality in two critical places. * The original re2j code supported only the Perl syntax for named capture groups "(?P<foo?)". Java supports only a slightly different syntax "(?<foo?)". The original port to scalanative changed changed the code to support only the Java syntax. This PR changes Parser.scala to support both idioms. This allows two re2j test cases for equals() and hashCode() to execute and pass. The change was documented, see below. Two java.util.regex Suites were edited to allow the now supported Perl syntax. * The unit-test Suite RegexpHashcodeEqualsTest.scala was added. * A section was added to `docs/lib/javalib.rst` to describe some RE2 Perl expressions now available in `java.util.regex`. These are not strictly Java 8 conformant but are likely to be useful to developers porting Perl regexen. Both the Java and Perl regular expressions for named capture groups are available. * Improved sn.regex named group support. * This PR ports [re2j](https://github.com/google/re2j/) PR #49, "Named group support", dated 2018-03-04, commit d0ec5a7cfec67a08735b720a956c92d1440b3789 to Scala Native. My thanks and appreciation to the re2j developer @sjamesr for the re2j PR. * Some named group support was activated/implemented when re2j was originally ported to Scala Native. This PR removes that code in favor to the quote "new" re2j named group support. I believe there is no major functional change. As more fully described below, a few Exceptions and their messages have been changed to conform to Java 8. My hope is that using some close relative of the re2j code will help future devos, myself included, identify and track relevant changes in the re2j repository. Requiescat in Pace, sn.regex named group support, we hardly knew ye! * Deviations of note from re2j code: + Adjusted Exception thrown and message to match Java 8. The re2j message was better in the sense that it gave the name not found, but SN is trying to hew close to Java 8. + Converted from namedGroups from Map[String, Integer] to Map[String, Int] to avoid boxing values which are known to be Ints. + Re-worked the named group methods start(name), end(name), and group(name) to use a common subroutine rather than open coding. This makes it easier to get the lookup & exception handling right and consistent. * unit-test points were ported/added: + sn.regex.MatcherSuite + sn.regex.ParserSuite + java.util.regex.MatcherSuite + java.util.regex.PatternSuite The majority of the testing of both Perl and Java syntax named group handling is done in sn.regex to stay close to the code which actually does the work. Add a probabilistic fix to "stack safe" test. This is a micro-edit point fix. A better comprehensive, deterministic fix is being exercised in work-in-progress for a future PR. Prior to this commit, the "stack safe" test could intermittently fail with duplicate groups. This fix increases the size of the string to drastically reduce the chance of a duplicate name. * named captures in sn.regex appendReplacement; parity with re2j release V1.3 * This PR addresses a number of PRs in the parent code [re2j] (https://github.com/google/re2j/). It brings sn.regex up to date with all relevant PRs in the re2j repository dated 2019-08-21, commit 86028a5d722956800c6d6257e713d539e1bdd24d. In particular, it incorporates all relevant PRs included in re2j Release V1.3, dated 2019-07-22, commit 222899bc5e058380935311eba419a722f00b3d69 + This PR is most closely related to re2j PR "Add named capture group support to appendReplacement " dated 2018-03-05, commit 0e87e2a2bb403edb4196698b6707eaa2c524cf96 It looks like some of that work had been done as part of the original port to sn.regex. This PR more closely aligns the work, adding test cases and such. + re2j PR "simplify Matcher.appendReplacement(StringBuffer, String)" dated 2019-03-05 is not relevant because the subject code was eliminated during the port to sn.regex because it was never used. + Other re2j PRs up to and including the top of the stack dated 2019-08-21 were considered and found to be not relevant. * My thanks and appreciation to the re2j developers for the re2j PRs. * Deviations of note from re2j code: + Some Exceptions were changed from IllegalArgumentException to the IllegalStateException used by Java 8. The corresponding messages were changed to match Java 8 as closely as feasible. In many case the re2j message was better because it gave the name of the duplicated group or such. For sn.regex, Java 8 alignment is more important. Some of the indices given by "near index X" are only true but the value of X may not be as good an approximation as on JVM. Similarly, sn.regex may echo a failing group in printStack() as only the near context which failed. Java 8 tends to give the entire string, making it easier to see a duplicate or such. Improvements for the next generation. * unit-test points were ported from re2j PR and additional test points created, particularly for Exception type and messages. * Add a deterministic fix to NamedGroupTest spuriously failing Before, the test could spuriously fail if two generated names ended up being duplicates. Now we check if the names repeat with the use of a Set. * Exclude incompatible mima checks Co-authored-by: Lee Tibbert <Lee Tibbert>

Don't create Machine while holding this' monitor.

6eea970

This change attempts to reduce monitor contention.

googlebot added the cla: yes label Feb 22, 2019

sjamesr merged commit fc83bf7 into master Feb 22, 2019

LeeTibbert mentioned this pull request Aug 23, 2019

nativelib regex RE2.Machine mutual exclusion improvements, re2j PR 85 & more scala-native/scala-native#1692

Closed

LeeTibbert mentioned this pull request Aug 31, 2019

Port/implement s.n.regex.Regexp hashCode(), equals(), & tests. scala-native/scala-native#1698

Closed

sjamesr deleted the no_machine_lock branch September 18, 2020 01:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't create Machine while holding this' monitor. #85

Don't create Machine while holding this' monitor. #85

sjamesr commented Feb 22, 2019

Don't create Machine while holding this' monitor. #85

Don't create Machine while holding this' monitor. #85

Conversation

sjamesr commented Feb 22, 2019