Skip to content

Inefficient codegen when extracting field from temporary object #23395

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.

Sign up for GitHub

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

Jump to bottom
Open
arnetheduck opened this issue Mar 12, 2024 · 4 comments · May be fixed by #24874
Open

Inefficient codegen when extracting field from temporary object #23395

arnetheduck opened this issue Mar 12, 2024 · 4 comments · May be fixed by #24874
Assignees
@ringabout
Labels
ARC/ORC Memory Management Status

Comments

@arnetheduck
Copy link
Contributor

Description

The following snippet, of critical performance importance when working with sum types, generates an inefficient copy, even though it can be seen that lifetimes are bounded in such a way that the copy can become a move:

type X = object
  s*: seq[byte]
proc f(): X = X(s: newSeq[byte](1024*1024*1024))
let v = f().s
	nimZeroMem((void*)(&T1_), sizeof(tyObject_X__4CKW0EgTl4SNofakrtBaFw));	_ZN6testit1fE((&T1_));
	genericSeqAssign((&v__testit_u35), T1_.s, (&NTIseqLbyteT__6H5Oh5UUvVCLiakt9aTwtUQ_));}

Notably, the same applies to any "last use" of a variable:

let 
  fv = f()
  v = fv.s # no need to copy if this is the last use of fv

Of course, in the real world, this applies most significantly to Option and its ilk:

proc f(): Option[seq[byte]] = ...
let
  x = f()
if x.isNone():
  return
let y = x.get() # last use of x - we can move value out of x
...

Nim Version

1.6, 2.0, devel....

Current Output

No response

Expected Output

No response

Possible Solution

No response

Additional Information

No response
@Araq
Copy link
Member

Araq commented Mar 14, 2024

Does it also happen with ORC?

@ringabout
Copy link
Member

Yeah, ORC is affected too, I checked before. In moveOrCopy, nkCallKinds is supplied with genSink. Perhaps we can treat the extracting fields etc. the same way as the nkCallKinds.

@ringabout ringabout self-assigned this Mar 14, 2024
ringabout added a commit that referenced this issue Mar 15, 2024
@ringabout
fixes #23395; Inefficient codegen when extracting field from temporar…
13206fd 
…y object
@ringabout ringabout mentioned this issue Mar 15, 2024
Closed
@ringabout ringabout mentioned this issue Jun 21, 2024
Open
@mratsim
Copy link
Collaborator

mratsim commented Jun 24, 2024 •
edited
Loading

Probably related to performance issues I had with Option and that I tried to fix by returning lent T: #14420 (comment)

The impact of Option was 5x slowdown compared to in-place mutation + bool

@arnetheduck
Copy link
Contributor Author

arnetheduck commented Jun 26, 2024 •
edited
Loading

lent is unrelated / orthogonal to this issue - it should should make no difference since the contents should be moved out which lent doesn't materially affect

@ringabout ringabout linked a pull request Apr 14, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ringabout ringabout

Labels
ARC/ORC Memory Management Status
Projects
None yet
Milestone
No milestone
4 participants
@Araq @arnetheduck @mratsim @ringabout