Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Minor performance improvement #120

Closed
wants to merge 1 commit into from

2 participants

@javiereguiluz

No description provided.

@michelf
Owner

Did you measure an actual gain in performance or are you just assuming it's better because you're using a built-in function?

@javiereguiluz

You are absolutely right! The common sense says that the proposed solution is much better, but I should have included some performance numbers.

I've prepared the following simple benchmark:

// My proposal
$start = microtime(true);
for ($i=0; $i<10000; $i++) {
  $classname = ".php";
  ltrim($classname, '.');
}
$end = microtime(true);

echo "ltrim() : " . 1000 * ($end - $start) . " ms \n";


// The original code
$start = microtime(true);
for ($i=0; $i<10000; $i++) {
  $classname = ".php";
  if ($classname{0} == '.')
        $classname = substr($classname, 1);
}
$end = microtime(true);

echo "if() + substr() : " . 1000 * ($end - $start) . " ms \n";

You can try the above code online at: http://sandbox.onlinephpfunctions.com/

As you can see, the ltrim() solution is 50% to 100% faster than if() + substr(). I know that if the code is executed a few times, the gain is negligible, but in my case, I use this code hundreds to thousands of times per script execution.

@michelf
Owner

Some tinkering with your synthetic benchmark shows that ltrim is faster when $classname starts with a dot, but slightly slower when there is no dot. The question is how often is there a dot?

Let's look at the part of the regex above capturing the class name:

\.?([-_:a-zA-Z0-9]+) # 2: standalone class name

Seems like the caching parenthesis is already excluding the dot, which means that $classname should never contain a dot at all. So we're optimizing something that doesn't need to be done in the first place.

@javiereguiluz

You are right again!

This morning I started integrating your library in my easybook project to replace the old dflydev-markdown library I currently use. As the codebase is pretty large and I define several different Markdown parsers and I was tweaking your fenced code blocks regexp, I guess that some class inheritance were wrong and I ended up having the . character in my $classname variable.

I'm glad that at least this pull request has been useful to detect some unneeded code :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Aug 8, 2013
  1. @javiereguiluz
This page is out of date. Refresh to see the latest.
Showing with 1 addition and 3 deletions.
  1. +1 −3 Michelf/Markdown.php
View
4 Michelf/Markdown.php
@@ -2801,9 +2801,7 @@ protected function _doFencedCodeBlocks_callback($matches) {
array(&$this, '_doFencedCodeBlocks_newlines'), $codeblock);
if ($classname != "") {
- if ($classname{0} == '.')
- $classname = substr($classname, 1);
- $attr_str = ' class="'.$this->code_class_prefix.$classname.'"';
+ $attr_str = ' class="'.$this->code_class_prefix.ltrim($classname, '.').'"';
} else {
$attr_str = $this->doExtraAttributes($this->code_attr_on_pre ? "pre" : "code", $attrs);
}
Something went wrong with that request. Please try again.