Conversation
1. Rational does not need a regexp to look for ".". 2. splitCommon can derive limit boolean. Change callers as appropriate.
limit == 1 logic moved to splitCommon
i variable passed in is confusing and not needed.
read all and read line were calculating CR but not setting it on the resulting string. This prevents resulting string from having single byte optimizable code paths work. I only did one benchmark using aref and it appears to be a little faster consistently but not a lot. I imagine using really long lines and a complicated backtracking regexp will make this pop.
|
@headius @lopex I am mostly looking for holes in RubyRegexp.isSimpleString. The rest I think is just having more hotpaths and reorganizing to make selecting those hot paths easier to select. The actual simple string logic should be very conservative in that lots of valid strings containing those chars are being rejected. The two fundamental questions are:
|
headius
left a comment
headius
left a comment
There was a problem hiding this comment.
The changes look fine, assuming discussions today about detecting simple regex have been considered or will be considered.
|
Is there code out there using |
|
@eregon You may be missing that when people do things like this I am not there to tell them they could do it better 😃 In the case I noticed this was someone submitted an issue where the regexp was not the specific issue but it still had: arr << s.split(/,/).join('+')I have seen people do stuff like |
|
@eregon Actually you just put me down a philosophical hole with this question. Why should a valid regular expression which is just a simple string be expected to be slower than just using the string? I admit I had the same reaction as you originally but part of me figured since it is correctable why not do it (note: I also did a hot path on ASCII string patterns at the same time so those also sped up)? |
3 participants
No description provided.