[std.regex] Bit-NFA kickstart engine by DmitryOlshansky · Pull Request #4286 · dlang/phobos

DmitryOlshansky · 2016-05-07T15:03:56Z

This adds an alternative "kickstart" engine to locate a prefix of a given pattern.
It is far more general the the original ShiftOr while a being few times slower,
still it's many times faster then any full-blow engine.

UPDATE: it's even ~x2 times faster then grep! Now that's some news :)

DmitryOlshansky · 2016-05-07T18:41:53Z

DmitryOlshansky · 2016-05-07T20:28:04Z

Win32 is blocked by https://issues.dlang.org/show_bug.cgi?id=15989

dlang-bot · 2016-10-06T22:26:20Z

Fix	Bugzilla	Description
✓	9391	Constant std.regex.regex

DmitryOlshansky · 2016-10-06T22:27:00Z

Now with immutable regexes!

aG0aep6G · 2016-10-06T22:31:49Z

Looks like you have a typo in the issue number. 9381 isn't about regex.

DmitryOlshansky · 2016-10-06T22:38:24Z

Looks like you have a typo in the issue number. 9381 isn't about regex.

Right my bad issue is 9391.

DmitryOlshansky · 2016-10-07T15:39:08Z

All green!

andralex

This is terrific work, and a large project on its own. I can't pretend to be able to review the logic of the additions in a reasonable time. Hopefully a paper or article will follow :o).

Two things:

Some lines that seem important are not covered in unittests.
Although this is in a way a project with its own life, it would be good to bring at least newly added code in alignment with the prevalent Phobos style. There, we use spaces around all operators; here, sometimes spaces are present but some other times they're omitted.

andralex · 2016-10-07T23:29:50Z

std/regex/internal/backtracking.d

            break;
        case IR.Trie:
-            if (charsets.length && charsets[ir[0].data].byInterval.length  <= 8)
+            if (charsets.length && charsets[ir[0].data].length  <= 8)


This line seems to be not covered.

Also has two spaces before <= :)

andralex · 2016-10-07T23:31:10Z

std/regex/internal/bitnfa.d

+                break;
+            slot += 1;
+            if (slot == table.length)
+                slot = 0;


andralex · 2016-10-07T23:31:35Z

std/regex/internal/bitnfa.d

+                break;
+            case LookaheadStart, NeglookaheadStart, LookbehindStart,
+                NeglookbehindStart:
+                paths.push(j + IRL!LookaheadStart + ir[j].data + IRL!LookaheadEnd);


uncovered (this would be important, right?)

andralex · 2016-10-07T23:31:45Z

std/regex/internal/bitnfa.d

+                i += (s-1)*IRL!OrChar;
+                bitCount++;
+                if (bitCount == 32)
+                    break outer;


andralex · 2016-10-07T23:31:54Z

std/regex/internal/bitnfa.d

+                finalMask |= 1u<<bitMapping[i];
+                break;
+            case Any:
+                uint mask = 1u<<bitMapping[i];


andralex · 2016-10-07T23:37:23Z

@DmitryOlshansky by the new flow you can merge your work now, optionally with follow-up work after the review.

schuetzm · 2016-10-08T11:33:01Z

std/regex/package.d

@@ -320,13 +320,11 @@ public alias StaticRegex(Char) = std.regex.internal.ir.StaticRegex!(Char);

    Throws: $(D RegexException) if there were any errors during compilation.
 +/


This doc comment needs to be moved down to regex(); now it refers to the newly inserted regexPure().

DmitryOlshansky · 2016-10-08T15:25:33Z

@andralex Will try to resolve the issues first then merge.

DmitryOlshansky · 2016-10-09T12:48:34Z

Auto-merge toggled on

MartinNowak · 2016-10-19T23:06:36Z

Seems like this caused a regression w/ building Higgs.
Issue 16626 – [Reg 2.073] recent dmd nightly runs out of memory building Higgs.

DmitryOlshansky · 2016-10-20T18:03:58Z

Seems like this caused a regression w/ building Higgs.
Issue 16626 – [Reg 2.073] recent dmd nightly runs out of memory building Higgs.

The new version is more demanding on memory during CTFE so yeah, most likely it now fails for some patterns. Dunno what to do here would really love to see the new engine sometime soon.

MartinNowak · 2016-10-23T22:51:32Z

Seems to me we should disable that engine in CTFE for now.
Let's continue the discussion in Bugzilla https://issues.dlang.org/show_bug.cgi?id=16626#c2.

- consumes too much memory, introduced by e98fa4a (dlang#4286)

MartinNowak · 2017-01-17T14:32:17Z

Seems to have caused another regression.
Issue 17066 – [REG2.073a] std.regex captures got immutable

DmitryOlshansky · 2017-01-17T16:25:41Z

@MartinNowak will look into it, though sounds highly suspicious.

DmitryOlshansky · 2017-01-17T21:16:32Z

@MartinNowak #5042

DmitryOlshansky force-pushed the bitnfa branch 4 times, most recently from 45da00b to 3577a07 Compare May 7, 2016 15:33

DmitryOlshansky force-pushed the bitnfa branch from 725e87b to 76aab06 Compare May 7, 2016 18:43

DmitryOlshansky mentioned this pull request May 7, 2016

[Refactor] Generalize kickstart engines, in preparation for more to come #4217

Closed

DmitryOlshansky force-pushed the bitnfa branch from 76aab06 to dda8b20 Compare May 7, 2016 20:02

DmitryOlshansky force-pushed the bitnfa branch from dda8b20 to 54c4d63 Compare May 8, 2016 09:42

9il added the std.regex label May 8, 2016

9il added the Merge:Blocked label May 23, 2016

DmitryOlshansky force-pushed the bitnfa branch 3 times, most recently from c747d99 to 9416854 Compare July 30, 2016 20:06

DmitryOlshansky added 15 commits October 1, 2016 20:59

[Refactor] Generalize kickstart engines, in preparation for more to come

66048ae

A start on bit-NFA

1416ddb

ASCII-only version of Bit-NFA

99095ee

[std.regex] Bit-NFA: implement unicode trie

ed4e07c

[std.regex] Improved hash-table for Bit-NFA

5c21564

[std.regex] Even faster hash table for Bit-NFA

7d930c1

[std.regex] Bit-NFA fix premature stop on repetitions

845551b

[std.regex] Bit-NFA - fix inversion of the right sub-portion of regex

4610231

[std.regex] Add Bit-NFA to win32/win64 makefiles

c27b118

[std.regex] Integrate Bit-NFA into std.regex

138a2f3

[std.regex] Limit ShiftOr to only apply where it's better then Bit-NFA

88ce1a1

Trailing whitespace

2011a1a

Try to reduce memory usage in CT-regex tests

20c5e39

WIP fixing multi-pattern match

3544686

[std.regex] Get Bit-NFA working on the testsuite

ecd33f1

DmitryOlshansky added 2 commits October 1, 2016 20:59

[std.regex] Save memory by building regex in 3 parts

3bed849

[std.regex] rebase on top of latest master, fix shiftor

91f0671

DmitryOlshansky force-pushed the bitnfa branch 2 times, most recently from cfe7a48 to e42788c Compare October 6, 2016 22:26

DmitryOlshansky added 2 commits October 7, 2016 01:40

[std.regex] Fix issue 9391 - const regex, ctRegex is immutable

e98fa4a

[std.regex] More tests for issue 9391

87847ed

DmitryOlshansky force-pushed the bitnfa branch from e42788c to 87847ed Compare October 6, 2016 22:41

DmitryOlshansky added 2 commits October 7, 2016 01:43

[std.regex] Trailing whites

cd2c28f

[std.regex] Fixes for recent compiler version

d1d53c5

andralex approved these changes Oct 7, 2016

View reviewed changes

andralex removed the Merge:Blocked label Oct 7, 2016

schuetzm reviewed Oct 8, 2016

View reviewed changes

[std.regex] Addressing review comments

f8b3eea

DmitryOlshansky merged commit 47adcab into dlang:master Oct 9, 2016

MartinNowak added a commit to MartinNowak/phobos that referenced this pull request Dec 24, 2016

disable kickstart in ctfe to workaround Issue 16626

07090ae

- consumes too much memory, introduced by e98fa4a (dlang#4286)

MartinNowak mentioned this pull request Dec 24, 2016

disable kickstart in ctfe to workaround Issue 16626 #4995

Merged

		@@ -320,13 +320,11 @@ public alias StaticRegex(Char) = std.regex.internal.ir.StaticRegex!(Char);

		Throws: $(D RegexException) if there were any errors during compilation.
		+/

Uh oh!

Conversation

DmitryOlshansky commented May 7, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DmitryOlshansky commented May 7, 2016

Uh oh!

DmitryOlshansky commented May 7, 2016

Uh oh!

dlang-bot commented Oct 6, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DmitryOlshansky commented Oct 6, 2016

Uh oh!

aG0aep6G commented Oct 6, 2016

Uh oh!

DmitryOlshansky commented Oct 6, 2016

Uh oh!

DmitryOlshansky commented Oct 7, 2016

Uh oh!

andralex left a comment

Choose a reason for hiding this comment

Uh oh!

andralex Oct 7, 2016

Choose a reason for hiding this comment

Uh oh!

andralex Oct 7, 2016

Choose a reason for hiding this comment

Uh oh!

andralex Oct 7, 2016

Choose a reason for hiding this comment

Uh oh!

andralex Oct 7, 2016

Choose a reason for hiding this comment

Uh oh!

andralex Oct 7, 2016

Choose a reason for hiding this comment

Uh oh!

andralex commented Oct 7, 2016

Uh oh!

schuetzm Oct 8, 2016

Choose a reason for hiding this comment

Uh oh!

DmitryOlshansky commented Oct 8, 2016

Uh oh!

DmitryOlshansky commented Oct 9, 2016

Uh oh!

MartinNowak commented Oct 19, 2016

Uh oh!

DmitryOlshansky commented Oct 20, 2016

Uh oh!

MartinNowak commented Oct 23, 2016

Uh oh!

MartinNowak commented Jan 17, 2017

Uh oh!

DmitryOlshansky commented Jan 17, 2017

Uh oh!

DmitryOlshansky commented Jan 17, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

DmitryOlshansky commented May 7, 2016 •

edited

Loading

dlang-bot commented Oct 6, 2016 •

edited

Loading