Repository navigation
Conversation
MLModel.compileModel drops the .mlmodelc bundle in the process's container-shared temp dir, ignoring TMPDIR overrides. Moving it next to the source .mlpackage allows hosts managing per-launch TMPDIR to reclaim it upon process termination, preventing disk leaks. - Derives destination from mlpackagePath as the single source of truth. - Fails hard if moveItem fails instead of falling back to leaked path. - Cleans up artifacts on failure; retains bundle on success for MLModel lifetime. Fixes lightvector#1266.
ChinChangYang
suggested changes
Oct 27, 2026
Comment on lines
+387
to
+400
| var unusedCompiledURL: URL? = compiledURL | ||
| defer { | ||
| if let unusedCompiledURL { | ||
| deleteSourceModel(at: unusedCompiledURL, serverThreadIdx: serverThreadIdx) | ||
| } | ||
| } | ||
| if stagedURL.standardizedFileURL != compiledURL.standardizedFileURL { | ||
| try FileManager.default.moveItem(at: compiledURL, to: stagedURL) | ||
| unusedCompiledURL = stagedURL | ||
| } | ||
|
|
||
| printError("Metal backend \(serverThreadIdx): Loading compiled model...") | ||
| let model = try MLModel(contentsOf: compiledURL, configuration: config) | ||
| let model = try MLModel(contentsOf: stagedURL, configuration: config) | ||
| unusedCompiledURL = nil |
Contributor
There was a problem hiding this comment.
- var unusedCompiledURL: URL? = compiledURL
- defer {
- if let unusedCompiledURL {
- deleteSourceModel(at: unusedCompiledURL, serverThreadIdx: serverThreadIdx)
- }
- }
- if stagedURL.standardizedFileURL != compiledURL.standardizedFileURL {
+ let modelURL: URL
+ do {
try FileManager.default.moveItem(at: compiledURL, to: stagedURL)
- unusedCompiledURL = stagedURL
+ modelURL = stagedURL
+ } catch {
+ printError("Metal backend \(serverThreadIdx): Warning: Failed to move compiled model to \(stagedURL.path), loading it from \(compiledURL.path): \(error)")
+ modelURL = compiledURL
}
printError("Metal backend \(serverThreadIdx): Loading compiled model...")
- let model = try MLModel(contentsOf: stagedURL, configuration: config)
- unusedCompiledURL = nil
+ let model = try MLModel(contentsOf: modelURL, configuration: config)
Deleting files on the failure paths is outside the agreed scope in steps 1-2 of #1266.
Author
There was a problem hiding this comment.
Updated in 5f6aacf — removed bundle deletion on the failure paths and fallback to loading from the original URL with a warning.
A failed move warns and loads from the compileModel URL instead of failing the load. Leave the compiled bundle for the caller or the system to reclaim.
ChinChangYang
suggested changes
Oct 28, 2026
ChinChangYang
left a comment
ChinChangYang
left a comment
Contributor
There was a problem hiding this comment.
Please update PR description to match the code.
Contributor
|
Thanks, both comments are addressed. The PR description still doesn't match the code; please replace the Changes bullets with:
|
Author
Updated the PR description as suggested, thanks! |
Contributor
|
I approved this PR. Two minor issues:
|
ChinChangYang
approved these changes
Oct 30, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MLModel.compileModel(at:)writes the compiled.mlmodelcintoNSTemporaryDirectory(),which ignores a
TMPDIRoverride. The source.mlpackageis created undertemp_directory_path(), which honorsTMPDIR. A host that gives each launch its ownTMPDIRand reclaims it on exit never sees the compiled bundle, so it leaks (about 50 MiBper ANE handle per launch).
Changes
compileModelreturns, move the bundle next to the source.mlpackageand load it from there. The destination ismlpackagePath.deletingLastPathComponent(); this does not look up the temp directory again.compileModelleft it.MLModelmay read it lazily. The owner of the temp directory reclaims it.Fixes #1266.