Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Save some CPU calculations #1271

Merged
merged 1 commit into from
Jan 19, 2017
Merged

Save some CPU calculations #1271

merged 1 commit into from
Jan 19, 2017

Conversation

mauroc8
Copy link

@mauroc8 mauroc8 commented Jan 1, 2017

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
New tests added? not needed
Fixed tickets
License MIT

Description

I just made the code more simple. It works exactly the same but now it's more readable, and the cpu has to make fewer calculations.

Let's compare:

textIndexOfEndOfFarthestNode = currentTextIndex + (newNode || currentNode).nodeValue.length +
    (newNode ? currentNode.nodeValue.length : 0) -
    1;

If newNode is defined, then it resolves to:

textIndexOfEndOfFarthestNode = currentTextIndex + newNode.nodeValue.length +
    currentNode.nodeValue.length - 1;

If newNode is undefined, it resolves to:

textIndexOfEndOfFarthestNode = currentTextIndex + currentNode.nodeValue.length - 1;

Then, it is more readable to write it like this:

textIndexOfEndOfFarthestNode = currentTextIndex + currentNode.nodeValue.length +
    (newNode ? newNode.nodeValue.length : 0) - 1;

Then there's the other line:

endSplitPoint = (newNode || currentNode).nodeValue.length -
    (textIndexOfEndOfFarthestNode + 1 - matchEndIndex);

Let's break it. When newNode is defined, it resolves to:

endSplitPoint = newNode.nodeValue.length -
    (textIndexOfEndOfFarthestNode + 1 - matchEndIndex);

Which is equivalent to:

endSplitPoint = newNode.nodeValue.length -
    textIndexOfEndOfFarthestNode - 1 + matchEndIndex;

Now, when newNode is defined, textIndexOfEndOfFarthestNode will be equal to currentTextIndex + currentNode.nodeValue.length + newNode.nodeValue.length - 1;

So we replace:

endSplitPoint = newNode.nodeValue.length -
    (currentTextIndex + currentNode.nodeValue.length + newNode.nodeValue.length - 1) -
    1 + matchEndIndex;

We remove parenthesis:

endSplitPoint = newNode.nodeValue.length - currentTextIndex -
    currentNode.nodeValue.length - newNode.nodeValue.length + 1
    - 1 + matchEndIndex;

We can simplify because we have newNode.nodeValue.length - newNode.nodeValue.length and 1 - 1, which both equal to 0.

 endSplitPoint = - currentTextIndex - currentNode.nodeValue.length +
     matchEndIndex;

Finally, we rearrange the elements.

endSplitPoint = matchEndIndex - currentTextIndex -
    currentNode.nodeValue.length;

What if newNode is undefined?

endSplitPoint = currentNode.nodeValue.length -
                (textIndexOfEndOfFarthestNode + 1 - matchEndIndex);

endSplitPoint = currentNode.nodeValue.length -
    textIndexOfEndOfFarthestNode - 1 + matchEndIndex;

Now textIndexOfEndOfFarthestNode will be currentTextIndex + currentNode.nodeValue.length - 1.

endSplitPoint = currentNode.nodeValue.length -
    (currentTextIndex + currentNode.nodeValue.length - 1) - 1 + matchEndIndex;

endSplitPoint = currentNode.nodeValue.length -
    currentTextIndex - currentNode.nodeValue.length + 1 -
    1 + matchEndIndex;

And simplifying we got:

endSplitPoint = matchEndIndex - currentTextIndex;

So, endSplitPoint can be defined as:

endSplitPoint = matchEndIndex - currentTextIndex -
    (newNode ? currentNode.nodeValue.length : 0);

If you think about it, you'll be able to understand it's logic. It is not an essential change. It only makes things slightly more simple and avoids headaches when trying to understand. It also makes it easier for the CPU.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 627c0b6 on mauroc8:master into ** on yabwe:master**.

@j0k3r j0k3r changed the title 5.22.2 Save some CPU calculations Jan 10, 2017
@j0k3r
Copy link
Contributor

j0k3r commented Jan 19, 2017

Thanks for that big explanation !

@j0k3r j0k3r merged commit 91dc47e into yabwe:master Jan 19, 2017
j0k3r added a commit that referenced this pull request Jan 19, 2017
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.

3 participants