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

Poor performance when using return-value optimization due to double reset, nil check, genericReset #13879

Closed
arnetheduck opened this issue Apr 5, 2020 · 1 comment
Milestone

Comments

@arnetheduck
Copy link
Contributor

arnetheduck commented Apr 5, 2020

Notice three things:

  • zeroMem happens in f2, then genericReset happens in f - these are redundant
  • nil check redundant - RVO should never end up with a nil variable here
  • genericReset is extremely slow, in and of itself, because it can't be analysed by C compiler
type X = object
  a, b, c: seq[int]

proc f(): X = X()

proc f2() =
  var x = f()

f2()
N_LIB_PRIVATE N_NIMCALL(void, f__KQQBzps5MRoTwkbIifdBMQ)(tyObject_X__KqwtR1mZdQ5eNccTtsN3pA* Result) {
        chckNil((void*)Result);
        genericReset((void*)Result, (&NTI__KqwtR1mZdQ5eNccTtsN3pA_));
}
N_LIB_PRIVATE N_NIMCALL(void, f2__7DOD66mksAP2O3HJgMd2CQ)(void) {
        tyObject_X__KqwtR1mZdQ5eNccTtsN3pA x;
        nimZeroMem((void*)(&x), sizeof(tyObject_X__KqwtR1mZdQ5eNccTtsN3pA));
        f__KQQBzps5MRoTwkbIifdBMQ((&x));
}
Nim Compiler Version 1.1.1 [Linux: amd64]
Compiled at 2020-03-29
Copyright (c) 2006-2020 by Andreas Rumpf

git hash: 8c719fce54544c8d14ce7c0e4639c974c1748668
active boot switches: -d:release
@mratsim
Copy link
Collaborator

mratsim commented Apr 7, 2020

Related: #8745

@Araq Araq added this to the 1.2.2 milestone May 18, 2020
Araq added a commit that referenced this issue May 19, 2020
@Araq Araq closed this as completed in b35d370 May 19, 2020
narimiran pushed a commit that referenced this issue May 20, 2020
* progress
* make tests green
* maybe we also want to reset pointers, dunno
* progress
* cleanup; fixes #13879 [backport:1.2]

(cherry picked from commit b35d370)
EchoPouet pushed a commit to EchoPouet/Nim that referenced this issue Jun 13, 2020
* progress
* make tests green
* maybe we also want to reset pointers, dunno
* progress
* cleanup; fixes nim-lang#13879 [backport:1.2]
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

3 participants