[C#] RegEx is traag

[Verwijderd]

Ik wil een tekst parsen. Dit doe ik met indexOf en substring etc. Nu dacht ik, laat ik eens met regular expressions aan de gang gaan. Ik had een keer begrepen dat dat wel snel scheen te zijn.

Dus ik aan de slag, maar nu blijkt dat mijn regEx langzamer is dan mijn indexOf en substring bewerkingen. Ok, het scheelt niet veel, op 40.000 geparste regels scheelt het 2 à 3 seconden, maar toch. (regex: 15 seconden, indexof + substring 12,5 seconden)

Het gaat om dit soort teksten:

"Timbaz: raises $0.60 to $2.30 and is all-in"
"styx2000: raises $0.04 to $0.08"

Ik parse deze tekst met deze regex:

C#:

Regex raiseRegex = new Regex("(?<name>.+)(: raises )(?<raiseBy>[\\$|\\d|\\.]+) to (?<raiseTo>[\\$|\\d|\\.]+)(?<allIn> and is all-in)?");

Is mijn regex slecht geschreven?
Is het goed geschreven maar gewoon traag?
Gebruik ik het verkeerd?
Of is het er niet voor bedoeld om zo gebruikt te worden?


En dezelfde code maar nu onder elkaar, voor de mensen die dat prettiger vinden:

code:

Regex raiseRegex = new Regex(
   "(?<name>.+)
    (: raises )
    (?<raiseBy>[\\$|\\d|\\.]+) to (?<raiseTo>[\\$|\\d|\\.]+)
    (?<allIn> and is all-in)?");

[ Voor 3% gewijzigd door Verwijderd op 10-01-2007 16:58 ]

[NMe]

Regular expressions zijn er AFAIK niet omdat ze per se sneller zijn dan zelf zoeken, maar omdat ze kortere code opleveren en soms ook leesbaarder zijn.

[PolarBear]

Probeer eens (VB.net, ken de precieze C# syntax niet)

Visual Basic .NET:

Dim raiseRegexp As New System.Text.RegularExpressions.Regex("jouw regexp", RegexOptions.Compiled)

Het gaat dan om de optie RegexOptions.Compiled.

[PhysicsRules]

Ik heb wel eens gelezen dat .+ heel traag is, en dat je beter ^\n+ kunt gebruiken.

[ Voor 9% gewijzigd door PhysicsRules op 10-01-2007 19:35 ]

[Soultaker]

PhysicsRules schreef op woensdag 10 januari 2007 @ 19:33:
Ik heb wel eens gelezen dat .+ heel traag is, en dat je beter ^\n+ kunt gebruiken.

Lijkt me flauwekul. (Sowieso is dat [^\n]+ dan).

Hoe gebruik je die reguliere expressie? Maak je 'm elke keer opnieuw, of maak je 'm één keer en pas je 'm dan op elke regel toe? Het tweede is de goede manier, het eerste waarschijnlijk vrij traag. Kun je wat meer code laten zien?

[MSalters]

Waarom "(: raises)" ? ":.{7}" doet bijna hetzelfde, alleen hoeft de matcher niet te checken of de " raises" staat waar die zou moeten staan. De overige literals zijn vergelijkbaar. Je $ kan alleen aan het begin van een bedrag voorkomen, waarom zoek je die overal? De \\. komt na \\d+ en voor \\d{2}.Die hoef je dus ook niet overal te zoeken.

[Verwijderd]

Goede tips!

Ik ga mijn code even aanpassen en dan laat ik weten of het al wat sneller gaat

[Verwijderd]

Soultaker schreef op woensdag 10 januari 2007 @ 20:36:
Hoe gebruik je die reguliere expressie? Maak je 'm elke keer opnieuw, of maak je 'm één keer en pas je 'm dan op elke regel toe? Het tweede is de goede manier, het eerste waarschijnlijk vrij traag.

Je hebt helemaal gelijk. Ik declareerde de variabele en expressie binnen de loop. Dat vertraagd enorm!

Uiteindelijk heb ik nu iets meer dan 100.000 geparste regels in 7,1 seconden met RegEx. Met indexOf en substring duurt 't 12,1 seconden. Dat zijn tenminste verschillen waar ik iets aan heb.


Dit is de code die ik nu gebruik:

C#:

Regex isRaiseRegex = new Regex(": raises ", RegexOptions.Compiled);
Regex raiseRegex = new Regex("(?<name>.+)(.{9})(?<raiseBy>\\$[0-9]+\\.*[0-9]{0,2}).{4}(?<raiseTo>\\$[0-9]+\\.*[0-9]{0,2})(?<allIn> and is all-in)?", RegexOptions.Compiled);

DirectoryInfo di = new DirectoryInfo("c://hh");
FileInfo[] rgFiles = di.GetFiles("*.txt");
foreach(FileInfo fi in rgFiles)
{
   String line;
   using (StreamReader sr = new StreamReader(fi.FullName))
   {
      while ((line = sr.ReadLine()) != null) 
      {
         if (isRaiseRegex.Match(line).Success)
         {
            Match m = raiseRegex.Match(line);

            if (m.Success)
            {
               string naam = m.Groups["name"].ToString();
               string raisedBy = m.Groups["raiseBy"].ToString();
               string raisedTo = m.Groups["raiseTo"].ToString();
               bool isAllIn = ( m.Groups["allIn"].Length > 0 );
            }
         }
      }
   }
}

En voor de leesbaarheid de regex code die niet het scherm af loopt:

code:

Regex("
   (?<name>.+)
   (.{9})
   (?<raiseBy>\\$[0-9]+\\.*[0-9]{0,2})
   .{4}
   (?<raiseTo>\\$[0-9]+\\.*[0-9]{0,2})
   (?<allIn> and is all-in)?",
   RegexOptions.Compiled);

[CyBoB]

Volgensmij doe je nogsteeds een Match teveel

code:

if (isRaiseRegex.Match(line).Success)
         {
            Match m = raiseRegex.Match(line); 
...

maak daar eens

code:

Match m = raiseRegex.Match(line); 
if (m.Success)
         {
...

Van

//edit: nu ik nog een keer kijk is het waarschijnlijk gewoon een copy&paste foutje ofzo

[ Voor 13% gewijzigd door CyBoB op 11-01-2007 12:26 ]

[Verwijderd]

CyBoB schreef op donderdag 11 januari 2007 @ 12:23:
Volgensmij doe je nogsteeds een Match teveel

code:

if (isRaiseRegex.Match(line).Success)
         {
            Match m = raiseRegex.Match(line); 
...

maak daar eens

code:

Match m = raiseRegex.Match(line); 
if (m.Success)
         {
...

Van

//edit: nu ik nog een keer kijk is het waarschijnlijk gewoon een copy&paste foutje ofzo

nee, de code is wel juist.

Ik check namelijk eerst het type van de regel. Ik controleer dus eerst of er geraised wordt, want er kan ook gebet worden, of gefold etc.

In het geval van mijn code is de flow als volgt:

regel 1: "hier staat een tekst" - Check isRaise -> false
regel 2: "hier staat nog een tekst" - Check isRaise -> false
regel 3: "Mannetje: raises $0.75 to $1" - Check isRaise -> true -> parse regel
regel 4: "hier staat een andere tekst" - Check isRaise -> false

Als ik nu de check isRaise weghaal, dan wordt het dit:
regel 1: "hier staat een tekst" -> parse regel
regel 2: "hier staat nog een tekst" -> parse regel
regel 3: "Mannetje: raises $0.75 to $1" -> parse regel
regel 4: "hier staat een andere tekst" -> parse regel

Dat is veel langzamer.

[CyBoB]

Oke sorry, stom van mij. Zag over het hoofd dat het 2 verschillende regex'en waren

* CyBoB schaamt zich