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

Performance regression with loop() in a class sub #3560

Closed
MasterDuke17 opened this issue Mar 20, 2020 · 7 comments
Closed

Performance regression with loop() in a class sub #3560

MasterDuke17 opened this issue Mar 20, 2020 · 7 comments

Comments

@MasterDuke17
Copy link
Contributor

This code:

class FOO {
	our proto sub foo($) {*};
	multi sub foo(Int $n) {
		my @a = ^$n;
		my $b;
		loop (my $i = 0; $i < $n; $i++) {
			$b = @a[$i]
		}
		$b
	}
	multi sub foo(Str $s) {
		foo($s.Int)
	}
}

say FOO::foo(1_000_000);
say now - INIT now

used to take ~1.3s, but after 48ce47e takes ~2.1s. This was discovered when benchmarking some changes to String::CRC32. The sample code use String::CRC32; say String::CRC32::crc32("abcdefghijklmnopqrstuvwxyz0123456789~!#^_+;:./>?" x 100_000), using cosimo/perl6-string-crc32#12, caused 4.8 million calls each to MVM_frame_find_lexical_by_name for &postfix:<++> and &infix:«<».

@MasterDuke17
Copy link
Contributor Author

https://gist.github.com/Whateverable/7e08b6856f9aa8c20358e7370961d1cb has the timings for releases.

@MasterDuke17
Copy link
Contributor Author

https://gist.github.com/MasterDuke17/ece191957c1ee47136650926da661c75 has the output of MVM_dump_bytecode(tc) at the top of MVM_frame_find_lexical_by_name when name is '&postfix:<++>'.

@jnthn
Copy link
Member

jnthn commented Mar 25, 2020

It seems it produces nicely optimized code...but then never manages to complete OSR and use it.

@jnthn
Copy link
Member

jnthn commented Mar 25, 2020

It's still not clear how this ever worked (and it might have done so accidentally...) but the problem is that a proto invokes the body with invokewithcapture, and OSR doesn't know how to find the args and callsite for doing an arg guard test.

@MasterDuke17
Copy link
Contributor Author

MasterDuke17 commented Mar 25, 2020 via email

@jnthn
Copy link
Member

jnthn commented Mar 25, 2020

I already have a patch that helps (the time it outputs goes from 3.76 to 0.70 on my slow home VM); just been checking it doesn't knock off OSR otherwise (seems not) and spectesting it.

@jnthn
Copy link
Member

jnthn commented Mar 25, 2020

Fixed with MoarVM/MoarVM@f2a52cf5d7.

@jnthn jnthn closed this as completed Mar 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants