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

Add Copy to some structs #1332

Open
CrazyboyQCD opened this issue Jul 20, 2024 · 5 comments
Open

Add Copy to some structs #1332

CrazyboyQCD opened this issue Jul 20, 2024 · 5 comments
Assignees

Comments

@CrazyboyQCD
Copy link

CrazyboyQCD commented Jul 20, 2024

Godbolt Link
As you can see in the asm output, even set opt-level = 3, if we don't add Copy to structs with all fields Copy derived, in clone() it generates more mov and large struct can't trigger memcpy, I suggest to add Copy to them to alleviate binary size bloated and performance problem.
List of structs that could add Copy (Simple search may miss):

  • Dav1dWarpedMotionParams
  • Dav1dSequenceHeader
  • Rav1dSequenceHeader
  • Dav1dSegmentationData
  • Rav1dSegmentationData
  • Dav1dSegmentationDataSet
  • Rav1dSegmentationDataSet
  • Dav1dLoopfilterModeRefDeltas
  • Rav1dLoopfilterModeRefDeltas
  • Rav1dFilmGrainData
  • Dav1dFilmGrainData
  • Dav1dFrameHeaderFilmGrain
  • Rav1dFrameHeaderFilmGrain
  • Dav1dFrameHeaderOperatingPoint
  • Dav1dFrameHeaderSuperRes
  • Rav1dFrameHeaderSuperRes
  • Dav1dFrameHeaderTiling
  • Rav1dFrameHeaderTiling
  • Dav1dFrameHeaderQuant
  • Rav1dFrameHeaderQuant
  • Dav1dFrameHeaderSegmentation
  • Rav1dFrameHeaderSegmentation
  • Dav1dFrameHeaderDeltaQ
  • Rav1dFrameHeaderDeltaQ
  • Dav1dFrameHeaderDeltaLF
  • Rav1dFrameHeaderDeltaLF
  • Dav1dFrameHeaderDelta
  • Rav1dFrameHeaderDelta
  • Dav1dFrameHeaderLoopFilter
  • Rav1dFrameHeaderLoopFilter
  • Dav1dFrameHeaderCdef
  • Rav1dFrameHeaderCdef
  • Dav1dFrameHeaderRestoration
  • Rav1dFrameHeaderRestoration
  • Dav1dFrameHeader
  • Rav1dFrameSize
  • Rav1dFrameSkipMode
  • Rav1dFrameHeader
  • Rav1dPictureParameters
  • AlignedPixelChunk
  • CdfContext
  • CdfMvContext
  • CdfMvComponent
  • CdfCoefContext
  • CdfModeContext
  • CdfModeInterContext
  • Rav1dTileGroupHeader
  • Av1BlockIntra
  • Av1BlockInter1d
  • Av1BlockInter2d
  • Av1BlockInterNd
  • Av1BlockInter
@kkysen
Copy link
Collaborator

kkysen commented Jul 22, 2024

I didn't realize this happens. Very good catch! We'll definitely look into this. It's a shame rustc optimizes non-Copy types so much worse, though, even when they're otherwise identical.

@CrazyboyQCD
Copy link
Author

Opened an issue.

@kkysen
Copy link
Collaborator

kkysen commented Sep 17, 2024

rust-lang/rust#128081 (comment) was fixed, at least in significant part, by rust-lang/rust#128299, so we should check if this is still an issue for our types.

@kkysen
Copy link
Collaborator

kkysen commented Oct 21, 2024

rust-lang/rust#128081 (comment) was fixed, at least in significant part, by rust-lang/rust#128299, so we should check if this is still an issue for our types.

This should land in 1.83 on 2024-11-28, so we should just wait until then and upgrade our toolchain.

@kkysen kkysen self-assigned this Oct 21, 2024
@CrazyboyQCD
Copy link
Author

rust-lang/rust#132356 disabled the optimization, should wait to be fixed.

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