Skip to content

Conversation

@pgovind
Copy link

@pgovind pgovind commented Apr 26, 2021

Apply #51508 to the One node.

Follow up:

  1. Update the Compiled code path
  2. Post perf numbers

Benchmarks have been added to dotnet/performance#1788

@pgovind pgovind marked this pull request as draft April 26, 2021 18:18
@ghost
Copy link

ghost commented Apr 26, 2021

Tagging subscribers to this area: @eerhardt, @pgovind
See info in area-owners.md if you want to be subscribed.

Issue Details

Apply #51508 to the One node.

Follow up:

  1. Update the Compiled code path
  2. Post perf numbers
Author: pgovind
Assignees: -
Labels:

area-System.Text.RegularExpressions

Milestone: -

if (len > i && _operator == RegexCode.Notoneloop)
{
TrackPush(len - i - 1, runtextpos - Bump());
Debug.Assert(_maxBacktrackPosition == -1, $"maxBacktrackPosition = {_maxBacktrackPosition}, runtext = {runtext}, runtextpos = {runtextpos}, ch = {ch}, code = {_code}, runregex = {runregex}");
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any assert failures stemming from this line are potential optimizations

// If lastIndexOf is -1, we backtrack to the max extent possible.
runtextpos = _maxBacktrackPosition;
ReadOnlySpan<char> runtextSpan = runtext.AsSpan(_maxBacktrackPosition);
int lastIndexOf = runtextSpan.LastIndexOf(str);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same questions as I had on the previous PR.

@pgovind pgovind added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Jun 4, 2021
@pgovind
Copy link
Author

pgovind commented Jun 4, 2021

This PR is still draft. Won't have time to work on it until #51508 (and its follow up work) goes in. Adding the no-merge label just to be extra clear about it. If somebody wants me to close the PR in the meanwhile, let me know.

@ghost ghost closed this Jul 4, 2021
@ghost
Copy link

ghost commented Jul 4, 2021

Draft Pull Request was automatically closed for inactivity. Please let us know if you'd like to reopen it.

@ghost ghost locked as resolved and limited conversation to collaborators Aug 3, 2021
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Text.RegularExpressions NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants