Skip to content

Refactor processing order#64

Merged
willdurand merged 2 commits into
willdurand:masterfrom
SL-Gundam:Refactor
Feb 7, 2018
Merged

Refactor processing order#64
willdurand merged 2 commits into
willdurand:masterfrom
SL-Gundam:Refactor

Conversation

@SL-Gundam

Copy link
Copy Markdown
Contributor

I found it incredibly annoying that lines were processed in reverse (horizontal reverse).

So i thought i'd modify that
It still works based on line order from the last to the first but the actual lines itself are normally readable

If this commit is desirable please merge this pull request. Otherwise just close it.
If any changes are desired please let me know

@SL-Gundam

Copy link
Copy Markdown
Contributor Author

Trying to find a good reason why my refactor might be a good reason to implement, since the results are the same either way (according to the tests), I did some performance benchmarks

Executing the function 10000 times in succession results in the following for various available test fixtures.
I executed the test 3 times for each fixture mentioned and averaged the result. Any results where in the three executions the margin between the 3 results was too big were redone
CPU: Intel 3700K
Windows 10 1709
IIS using FastCGI PHP 7.2.1
Extensions Xdebug and Zend OPcache enabled

correct_sig_1.txt
Current: 0.70 seconds
Refactor: 0.70 seconds

email_1.txt
Current: 2.11 seconds
Refactor: 1.89 seconds

email_2.txt
Current: 10.08 seconds
Refactor: 8.22 seconds

email_3.txt
Current: 6.69 seconds
Refactor: 6.48 seconds

email_bullets.txt
Current: 5.32 seconds
Refactor: 4.13 seconds

email_custom_quote_header.txt
Current: 2.85 seconds
Refactor: 2.50 seconds

email_sig_delimiter_in_middle_of_line.txt
Current: 1.70 seconds
Refactor: 1.38 seconds

While its unlikely that any script would perform these 10000 times in short succession, it does say that the refactor gives, in general, a slight performance improvement

I actually feared the refactor might perform worse. I'm glad that is not the case. I also was expecting the most time to be lost on the regexes (which would perform the same in both versions). So this much time gained was unexpected

@willdurand

Copy link
Copy Markdown
Owner

This is very cool, thank you for that!

@willdurand willdurand merged commit 8308e02 into willdurand:master Feb 7, 2018
@SL-Gundam SL-Gundam deleted the Refactor branch February 7, 2018 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants