-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Static-ness is not conserved for asset paths in AssetServer::load() #19844
Copy link
Copy link
Closed
Labels
A-AssetsLoad files from disk to use for things like images, models, and soundsLoad files from disk to use for things like images, models, and soundsC-BugAn unexpected or incorrect behaviorAn unexpected or incorrect behaviorC-PerformanceA change motivated by improving speed, memory usage or compile timesA change motivated by improving speed, memory usage or compile timesS-Needs-ReviewNeeds reviewer attention (from anyone!) to move forwardNeeds reviewer attention (from anyone!) to move forward
Milestone
Description
The changes in #10823, specifically the move to impl<'a> From<&'a str> for AssetPath<'a>, make us lose the benefit of CowArc::Static in the common case of asset_server.load("path"). With the new impl<'a> From<&'a str> for AssetPath<'a> approach we're statically erasing the 'static context. This means path.clone() (which we do a number of times in asset_server.load()) now always allocates an Arc<str> internally when we pass in a &'static str.
Given that this is the primary scenario CowArc was built, I believe it is worth fixing. The impl From<&'static str> for AssetPath<'static> was an intentional tradeoff to ensure asset_server.load("path") captures the static lifetime and prevents a clone.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
A-AssetsLoad files from disk to use for things like images, models, and soundsLoad files from disk to use for things like images, models, and soundsC-BugAn unexpected or incorrect behaviorAn unexpected or incorrect behaviorC-PerformanceA change motivated by improving speed, memory usage or compile timesA change motivated by improving speed, memory usage or compile timesS-Needs-ReviewNeeds reviewer attention (from anyone!) to move forwardNeeds reviewer attention (from anyone!) to move forward