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

regex match #1346

Merged
merged 1 commit into from
Mar 1, 2017
Merged

regex match #1346

merged 1 commit into from
Mar 1, 2017

Conversation

rebornix
Copy link
Member

@rebornix rebornix commented Mar 1, 2017

Use regex to match brackets instead of iterating all characters one by one. Fix performance issues of inserting brackets in a large file.

cc @xconverge @johnfn

@rebornix rebornix merged commit 406f3c1 into VSCodeVim:master Mar 1, 2017
@johnfn
Copy link
Member

johnfn commented Mar 2, 2017

Haha okay this is really interesting. I don't doubt that it's faster, but I have no idea why. A regex would certainly be slower than a handrolled checking function...

My best guess is that Position.IterateDocument was SUPER slow? That's kind of weird though.

@rebornix
Copy link
Member Author

rebornix commented Mar 2, 2017

@johnfn it's very likely Generators, which might not be optimized enough by V8.

@rebornix
Copy link
Member Author

rebornix commented Mar 2, 2017

	function norm() { 
		for (var i = 0; i < 10000000; i++);
		return 42;
	} 
	function *g() {
		for (var i = 0; i < 10000000; i++) {
			yield i;
		}
	}
	function gen() {
		for(const i of g());
		return 42;
	}
	console.time('normal');
	console.log(norm()); 
	console.timeEnd('normal');
	console.time('generator');
	console.log(gen());
	console.timeEnd('generator');

Result

generators.html:21 normal: 6.246ms
generators.html:24 generator: 578.637ms

@johnfn
Copy link
Member

johnfn commented Mar 3, 2017

Holy smokes. Yeah, that will do it!

This pull request was closed.
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