Cut ~59s from every offline install by not shelling out to Get-Disk - #66
Merged
Conversation
_disk_number() resolved the attached VHD's disk index with
`powershell -NoProfile -Command "(Get-Disk | ...).Number"`. Measured on a real
instance, that call costs ~17s -- almost none of it disk work. It is PowerShell
cold-loading the Storage/CIM providers on every spawn, and _Attached pays it
twice: once to resolve the device, once when _detach verifies the disk is gone.
Per _Attached block:
diskpart attach vdisk 1.6s <- the actual disk work
_disk_number (Get-Disk) 17.6s
_partition_offset 0.05s
detach (Get-Disk again) 19.0s
An install attaches Root.vhd and Data.vhdx separately, so a user waited ~76s,
about 70s of which was Get-Disk. For contrast, the e2fsck integrity pass that
looked like the expensive step is 0.27s -- it scales with used inodes and
blocks (~4400 files), not image size.
diskpart already knows the answer and prints it as `Associated disk#: 2`, so ask
it instead: one diskpart run, ~1.6s. Verified to agree with Get-Disk in both
states, attached and detached -- the detached case matters because _detach()
treats None as proof the disk released.
`Associated disk#` is English diskpart text, so the parse reports whether it
understood the output at all and we fall back to Get-Disk when it did not. A
localised Windows keeps working at the old speed rather than silently reading
"attached" as "gone", which would let _detach() claim success on a still-mounted
image.
Measured through the real code path, full attach/detach cycle:
Data.vhdx ~38s -> 8.55s
Root.vhd ~38s -> 8.20s
per install ~76s -> 16.75s
Full suite: 307 passed.
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a98c7e257
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+217
to
+218
| if _ASSOC_DISK_FIELD_RE.search(out): | ||
| return None, True # field present, no number => detached |
There was a problem hiding this comment.
On localized Windows where the BlueStacks data directory contains the literal Associated disk# (a valid Windows folder name), detail vdisk still prints that text as part of the VHD filename even though the actual field label is translated. This unanchored search therefore marks the output as parsed and returns None instead of falling back to Get-Disk, causing attached disks to be reported as missing and potentially making _detach() report success after a failed detach. Match a complete field line, including its delimiter, rather than any occurrence in the output.
Useful? React with 👍 / 👎.
Owner
Author
|
… On Tue, Jul 28, 2026 at 2:07 PM chatgpt-codex-connector[bot] < ***@***.***> wrote:
***@***.***[bot]* commented on this pull request.
💡 Codex Review
Here are some automated review suggestions for this pull request.
*Reviewed commit:* 2a98c7e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo
<https://chatgpt.com/codex/cloud/settings/general>. Reviews are triggered
when you
- Open a pull request for review
- Mark a draft as ready
- Comment ***@***.*** <https://github.com/codex> review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting ***@***.***
<https://github.com/codex> address that feedback".
------------------------------
In ext4_symlink.py
<#66 (comment)>
:
> + if _ASSOC_DISK_FIELD_RE.search(out):
+ return None, True # field present, no number => detached
*[image: P2 Badge]
<https://camo.githubusercontent.com/f2c1aacb361ddd3a0e9f9cacdb84fab050de434017f6747bb916e31e29bdf03d/68747470733a2f2f696d672e736869656c64732e696f2f62616467652f50322d79656c6c6f773f7374796c653d666c6174>
Anchor the disk field match to its output line*
On localized Windows where the BlueStacks data directory contains the
literal Associated disk# (a valid Windows folder name), detail vdisk
still prints that text as part of the VHD filename even though the actual
field label is translated. This unanchored search therefore marks the
output as parsed and returns None instead of falling back to Get-Disk,
causing attached disks to be reported as missing and potentially making
_detach() report success after a failed detach. Match a complete field
line, including its delimiter, rather than any occurrence in the output.
Useful? React with 👍 / 👎.
—
Reply to this email directly, view it on GitHub
<#66?email_source=notifications&email_token=ACS7QNSTFXXQIBN3W7HYJV35HD2XXA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIOBQGA4TKNJQGY32M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#pullrequestreview-4800955067>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACS7QNSMUOLAZMLXCOKD5AD5HD2XXAVCNFSNUABFKJSXA33TNF2G64TZHM4TCMJYHA3TAMZUHNEXG43VMU5TKMBQGIYDSNRSGY3KC5QC>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Summary
Testing
|
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
1 participant
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.
The finding
_disk_number()resolved the attached VHD's disk index viapowershell -NoProfile -Command "(Get-Disk | ...).Number". Measured on a realinstance, that one call costs ~17s -- and almost none of it is disk work. It is
PowerShell cold-loading the Storage/CIM providers on every spawn.
_Attachedpays it twice: once to resolve the device, once when_detachverifies the disk is gone. Broken down, per
_Attachedblock:diskpart attach vdisk(the actual disk work)_disk_number()→Get-Disk_partition_offset()(MBR read)detach(calls_disk_numberagain)An install attaches
Root.vhdandData.vhdxseparately, so the user waited~76s, roughly 70s of it in
Get-Disk.Worth recording the thing that wasn't slow: the
e2fsckintegrity pass, whichlooked like the obvious suspect, is 0.27s. It scales with used inodes and blocks
(~4,400 files), not image size. Removing it would save nothing and cost the check
that catches a bad
debugfswrite.The change
diskpartalready knows the answer and prints it asAssociated disk#: 2. Ask itinstead -- one diskpart run, ~1.6s. Verified to agree with
Get-Diskin bothstates:
Get-DiskThe detached case is the one that matters --
_detach()treatsNoneas proof thedisk released, so a fast path that returned
Nonefor an attached disk would letdetach report success on a still-mounted image.
Associated disk#is English diskpart text, so the parser reports whether itunderstood the output at all, and we fall back to
Get-Diskwhen it did not. Alocalised Windows keeps working at the old speed instead of failing silently.
Result
Measured through the real code path, full attach/detach cycle:
~59s off every install, and again off every uninstall.
Tests
tests/test_disk_number.pypins the behaviour that made the fast path safe: agreementwith
Get-Diskattached and detached, and -- most importantly -- that unrecognisedoutput falls back rather than being mistaken for "detached".
Full suite: 307 passed.