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

Still slower comparing to previouse Zen Coding #2

Closed
ArkadiuszMichalski opened this issue Feb 26, 2016 · 22 comments
Closed

Still slower comparing to previouse Zen Coding #2

ArkadiuszMichalski opened this issue Feb 26, 2016 · 22 comments

Comments

@ArkadiuszMichalski
Copy link
Contributor

I make more test and see that this version is significantly slower when comparing to Zen Coding 0.7 + NppScripting (older version of jN). Still can be downloaded from http://docs.notepad-plus-plus.org/index.php?title=Plugin_Central#_188
For bigger files the differences are clearly in favor of the older implementation (especially for Expand Abbrevation). I check this for files with 500, 1000, 1500, 2000 lines and run command on last line. In future is there any chance to improve speed?

@eight04
Copy link
Owner

eight04 commented Feb 26, 2016

I can confirm this. The expanding speed depends on the file size. But this issue is different from emmetio/npp#2

If you try:

... (more than 1000 lines) ...
.hello <- expand this

It takes about 2~3 seconds. But:

... (more than 1000 lines) ...
<div>
    .hello <- expand this
</div>

It can be expanded instantly.

I have to dig in emmet code to find out where is the bottleneck.

BTW, jN has builtin zen coding script. It is much faster than emmet too.

@ArkadiuszMichalski
Copy link
Contributor Author

Speed is not problem for more nested markup. But if cursor is totally outside or directly in <html> or even<body> (somewhere in the end) it is worse:

<html>
<body>
... (more than 1000 lines) ...
    <div>
        here is fast
    </div>
here is slow
</body>
here is slow
</html>
here is slow
<div>
here is fast
<div>

@eight04
Copy link
Owner

eight04 commented Feb 27, 2016

@ArkadiuszMichalski
Copy link
Contributor Author

Hmm in file 1,2k lines (expand in the end) I get 3 bottleneck. Please put alert like I did and you will see that slow is between:
alert1 and alert2
alert3_1 and alert3_2
alert3_3 and alert3_4

Code:

        expandAbbreviationAction: function(editor, syntax, profile) {
            var args = utils.toArray(arguments);alert("1");

            // normalize incoming arguments
            var info = editorUtils.outputInfo(editor, syntax, profile);alert("2");
            args[1] = info.syntax;
            args[2] = info.profile;

            return handlers.exec(false, args);
        },
    handlers.add(function(editor, syntax, profile) {
        var caretPos = editor.getSelectionRange().end;
        var abbr = findAbbreviation(editor);alert("3");

        if (abbr) {
            var content = parser.expand(abbr, {
                syntax: syntax, 
                profile: profile, 
                contextNode: actionUtils.captureContext(editor)
            });alert("4");

            if (content) {
                var replaceFrom = caretPos - abbr.length;
                var replaceTo = caretPos;

                // a special case for CSS: if editor already contains
                // semicolon right after current caret position — replace it too
                var cssSyntaxes = prefs.getArray('css.syntaxes');
                if (cssSyntaxes && ~cssSyntaxes.indexOf(syntax)) {
                    var curContent = editor.getContent();
                    if (curContent.charAt(caretPos) == ';' && content.charAt(content.length - 1) == ';') {
                        replaceTo++;
                    }
                }

                editor.replaceContent(content, replaceFrom, replaceTo);alert("5");
                return true;
            }
        }
        captureContext: function(editor, pos) {alert("3_1");
            var allowedSyntaxes = {'html': 1, 'xml': 1, 'xsl': 1};
            var syntax = editor.getSyntax();alert("3_2");
            if (syntax in allowedSyntaxes) {
                var content = editor.getContent();alert("3_3");
                if (typeof pos === 'undefined') {
                    pos = editor.getCaretPos();
                }

                var tag = htmlMatcher.find(content, pos);alert("3_4");
                if (tag && tag.type == 'tag') {
                    var startTag = tag.open;
                    var contextNode = {
                        name: startTag.name,
                        attributes: [],
                        match: tag
                    };

                    // parse attributes
                    var tagTree = xmlEditTree.parse(startTag.range.substring(content));
                    if (tagTree) {
                        contextNode.attributes = tagTree.getAll().map(function(item) {
                            return {
                                name: item.name(),
                                value: item.value()
                            };
                        });
                    }
                    alert("3_5");
                    return contextNode;
                }
            }

            return null;
        },

@eight04
Copy link
Owner

eight04 commented Feb 27, 2016

            var info = editorUtils.outputInfo(editor, syntax, profile);alert("2");

outputInfo uses editor.getSyntax too:
https://github.com/emmetio/emmet/blob/master/lib/utils/editor.js#L57

@ArkadiuszMichalski
Copy link
Contributor Author

Yep now see you find the same things:)

@eight04
Copy link
Owner

eight04 commented Feb 27, 2016

@eight04
Copy link
Owner

eight04 commented Feb 28, 2016

I have ported the old matcher to emmet: https://github.com/eight04/emmet/tree/dev-matcher-zen-coding
You can try this branch for the dist files: https://github.com/eight04/jn-npp-emmet/tree/zen-coding-matcher

The old matcher should be 2x faster, but there are some tests that it couldn't pass. I will checkout if I can fix it without a performance decrease. I will also try to cache the matching result. It should be 3x faster.

@ArkadiuszMichalski
Copy link
Contributor Author

Now is better for 1k line, but for more (like 3k or 4k) is still problem (but in old ZC is the same). In practice the most imprtant is expanding directly before </body>, before or after </html> does not matter (should not there be anything, so can be slowly). So maybe caching will helps more.

All news and good things from actual matcher (https://github.com/emmetio/emmet/blob/master/lib/assets/htmlMatcher.js) was ported?

@eight04
Copy link
Owner

eight04 commented Feb 28, 2016

What are news and good things?

Here is my implementation: https://github.com/eight04/jn-npp-emmet/tree/better-matcher

Some changes:

  • Use String.indexOf to search html token.
  • Search both direction instead of searching upward and downward.
  • Cache the result.

Maybe it can be faster, by searching the closing tag first, then directly find open tag with lastIndexOf.

@ArkadiuszMichalski
Copy link
Contributor Author

I'm mean infos from comment:

/**
 * HTML matcher: takes string and searches for HTML tag pairs for given position 
 * 
 * Unlike “classic” matchers, it parses content from the specified 
 * position, not from the start, so it may work even outside HTML documents
 * (for example, inside strings of programming languages like JavaScript, Python 
 * etc.)
 */

This last version is acceptable, in 4k lines delay is not so much as in master branch. Unfortunately two Balance options stop works.

@eight04
Copy link
Owner

eight04 commented Feb 28, 2016

That bug is fixed via 1e1b0f8

And yes, the matcher will search from current position, including zen coding one and mine.

@eight04
Copy link
Owner

eight04 commented Feb 29, 2016

Another implementation: https://github.com/eight04/jn-npp-emmet/tree/quick-match

It is faster than I expected 😮

@sieukrem
Copy link

When you use jN, take care about access to properties depending from text like View.text,View.anchor, View.pos.
All of them make internally a transformation from UTF-8 (Scintilla) to UTF-16(JavaScript).
There is no way to improve performance of View.text internally only by reducing of count of calls. For View.anchor and View.pos there are replacements View.byteAnchor and View.bytePos. They can be used if a character position is not necessary.

jN API

@ArkadiuszMichalski
Copy link
Contributor Author

@eight04 This last is realy fast when we work on the end of document, but at the beginning slows. Even outsiade html, which was not happend in better-matcher branch.

here is slow
<html>
</html>
here is fast

Using some Workers can help in this situation? In HTML files (if its big) you can open more Workers and use different algoritms to find open/close tag, the fastest will returns value and others can be close. Additionally we will not have the locked UI (like now when action takes some time).

@eight04
Copy link
Owner

eight04 commented Mar 1, 2016

I updated the algorithm and it should be better now. After some testing I will merge it into better-matcher branch, which will search both direction.

I don't think WSH support web worker.

@eight04
Copy link
Owner

eight04 commented Mar 1, 2016

@sieukrem Emmet's author has mentioned this too (emmetio/npp#2). But actually I don't feel any delay in jN comparing to emmetio/npp plugin.

Also, when can I use byteAnchor or bytePos? We can't access document in bytes right?

@sieukrem
Copy link

sieukrem commented Mar 1, 2016

Look for usages of pos and anchor. I have seen a usage for save and restore of pos and anchor. In this case it does not make any difference if you use byte or character positions, but byte is faster.

@ArkadiuszMichalski
Copy link
Contributor Author

Dialog has access to Worker interface, I check this in your code (line ~109) but don't test it in more ways:

document = d.document;
alert(document.parentWindow.Worker)

This last version is really fast, 4k lines its not a problem:).

@ArkadiuszMichalski
Copy link
Contributor Author

Unfortunately Balance (outward) option not work correctly, its stops after some invoking, never select body or html or other block elements (below select content beetwen ||).

<ul>
    |<li>
        <a href="">start balancing outward here</a>
    </li>|
</ul>

@eight04
Copy link
Owner

eight04 commented Mar 1, 2016

12a73c7
I should refactor those variable name...

@eight04
Copy link
Owner

eight04 commented Apr 3, 2016

I have released v1.0.0 with https://github.com/eight04/emmet/tree/dev-quick-match-2. The matcher will try not to scan the whole file but looking for nearing tag. It is faster if the cursor is nearing some tags having unique name.

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

No branches or pull requests

3 participants