-
Notifications
You must be signed in to change notification settings - Fork 5
#95 Implement cache purging (admin + non-admin) with timing + logging #97
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
base: dev
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice this appears to be a pure swing panel, no generated code! awesome :)
| } | ||
| } | ||
|
|
||
| public static void restartAsAdmin() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may want to indicate this is only windows since i see powershell syntax here, should warn that generally i don't think it's possible to do privilege escalation but if there are tricks that allow us to escalate easily that would be good knowledge for all of us. I think the normal approach is we check if it is an admin process and if not we don't attempt to drop the cache and inform the user that they must drop it manually. actually i think if they are not admin we just use your new disk cache purge function right? or is this method used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vpstackhub what version of netbeans are you using, interesting that this got checked in which seems to break running in netbeans for others?
specifically this line because it does not exist for other users.
project.dir=C:/Users/valer/OneDrive/Documenti/NetBeansProjects/jdm-java
Might you know why these attributes are being added into the project file?
| } | ||
| } | ||
|
|
||
| public static boolean dropOsCache() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vpstackhub is this the more robust version of Gui.dropCache()? If so it is valid to treat macos and linux the same which Gui.dropCache() does not?
|
@vpstackhub Looks really good thanks to your pull request I was able to follow some of the action! Two key items to look at are:
I think progress bar and some message notification in the log area is a responsible way to inform the user of the underlying operation. If running in command line mode we can output to the console that we are purging and there are ways to adjust the command line progress bar as well. Good progress on this objective which will make packaging requirements less painful and more robust for @jslcom @tylermlui and me. <3<3<3 :) |
|
|
||
| // Detect purge size roughly equal to the full test data footprint | ||
| long estimatedBytes = (long) App.numOfSamples * (long) App.numOfBlocks * | ||
| ((long) App.blockSizeKb * 1024L); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing that we might run into is if the bytes we purge is less than the actual disk cache the benchmark would still be affected by the disk cache. When i asked gemini it recommended purging the size of the entire main memory to be safe as that was the only way to guarantee that the disk cache does not affect our benchmark even if the disk cache is usually just a portion of main memory and for this reason if we go the safe route the the process might be a little lengthy since different system could have different amounts of memory installed, might be worth notifying user what is it doing so they understand why the clearing is taking so long. maybe we can default to purging the size of system memory + 10% (need to detect it from ), i think it's simply Runtime.getRuntime().totalMemory(); and maybe later can add an advanced option for the user to do the cache purging based on a percentage of the main memory if we find safe values for this. @jslcom are you familiar with the default disk cache sizes?
src/jdiskmark/Util.java
Outdated
| return (path.delete()); | ||
| } | ||
|
|
||
| public static void readPurge(long estimatedBytes) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you think about purgeDiskCache()?
Is it possible to read the disk cache to achieve purging? No writing is needed? I was under the impression we had to write the amount of the disk cache. Or is the code writing but it just wasn't obvious to me?
nbproject/project.properties
Outdated
| src.dir=src | ||
| test.src.dir=test | ||
| run.working.dir=${project.dir} | ||
| work.dir=${project.dir} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vpstackhub to run the project i needed to remove these lines but I'm not sure if the cache is executing. Possible we can meetup this week so I can show you what I'm seeing? Want to make sure I'm running the branch correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
val if you replace these lines does everything still work for you? if so maybe we can remove them or maybe can let me know what these lines will do for us?
| */ | ||
| /** | ||
| * Initialize the GUI Application. | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is super minor but lines 173-174 lost a valid space.
| cachePurgePerformed = false; | ||
| cachePurgeSizeBytes = 0L; | ||
| cachePurgeDurationMs = 0L; | ||
| cachePurgeMethod = CachePurgeMethod.NONE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| } | ||
|
|
||
| private void printCliProgress(int pct) { | ||
| int bars = pct / 5; // 20-bar progress |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
super minor, there is a java practice where the recommendation is to avoid abbreviations when possible so percent instead of pct.
| Process p = Runtime.getRuntime().exec("net session"); | ||
| p.waitFor(); | ||
| return p.exitValue() == 0; | ||
| } catch (Exception ex) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally catching Exception is work in progress / temporary code and RuntimeException less so. might be safe to catch RuntimeException here instead to avoid the appearance of eating checked exception.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks James,appreciate the thorough review. To reduce the back and forth, here’s a consolidated checklist (maybe more helpful for me than it is for you 😂) of what’s done and what I’ll follow up with next so we’re fully aligned.
⸻
Confirmed / Already Addressed
• [x] Removed NetBeans-specific project settings
Deleted run.working.dir and work.dir from nbproject/project.properties.
After removal, the project runs correctly on my side and cache purging still executes as expected. I agree these lines should not be committed.
• [x] Replaced purge method string literals with enum
Introduced Benchmark.CachePurgeMethod and updated all usages (GUI + CLI).
• [x] Resolved local DB enum mapping issue
Cleared .jdm/0.6.3-dev to regenerate schema cleanly --no further persistence errors.
• [x] User feedback during purge
• GUI log messages indicating purge start/end + method used
• CLI progress bar output when running in command line mode
⸻
Minor Cleanup / Follow-ups (next commit)
• [x] Remove duplicate Javadoc block + restore spacing in App.java
• [x] Rename pct → percent in BenchmarkWorker.printCliProgress() for readability
• [x] Narrow catch (Exception) -> catch (RuntimeException) in UtilOs.isProcessElevated()
• [x] (Optional) Move default cachePurge* values to field declarations in Benchmark
⸻
Once the above cleanup is in, I believe PR #95 should be fully ready from my side.
this should make cache behavior much better
| BenchmarkType benchmarkType; | ||
| public BenchmarkType getBenchmarkType() { return benchmarkType; } | ||
|
|
||
| public enum CachePurgeMethod { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vpstackhub I was thinking how about calling the top level operation CacheResetMethod this way if we know we need to do a cache reset based on the user being admin or not we can either DROP (admin removal) or PURGE (cache overwrite). This way the parent operation is called something different from the actual mechanism used.
public enum CacheClearanceMethod {
NONE,
DROP, // Requires Admin
PURGE // Fallback for Non-Admin
}
alternative could use CacheClearanceMethod or CacheMitigationMethod.
| Gui.mainFrame.refreshWriteMetrics(); | ||
| } | ||
|
|
||
| System.out.println(">>> BenchmarkWorker: ENTERING CACHE PURGE SECTION"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe this could be used: ENTERING CACHE RESET SECTION or ENTERING CACHE MITIGATION SECTION
|
|
||
| long purgeStartNs = System.nanoTime(); | ||
| benchmark.cachePurgePerformed = true; | ||
| benchmark.cachePurgeMethod = App.isAdmin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
potentially cachePurgePerformed > cacheResetPerformed
and cachePurgeMethod > cacheResetMethod
| // Detect purge size roughly equal to the full test data footprint | ||
| long estimatedBytes = (long) App.numOfSamples * (long) App.numOfBlocks * | ||
| ((long) App.blockSizeKb * 1024L); | ||
| benchmark.cachePurgeSizeBytes = estimatedBytes; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is set from zero to the actual purge size and only if not admin and a purge operation occurs.

Summary
This PR implements the cache purge feature requested in Issue #95, providing both:
Admin-level purge via EmptyStandbyList.exe
Fallback non-admin purge (PowerShell write-flush trick)
Detailed timing logs for James to review the UX impact
Clear console debug messages to distinguish pathways
No GUI changes yet. Waiting for James' preference on how to present timing (progress bar, spinner, modal, etc.)
Test Results
TEST A -- Running inside NetBeans (non-admin)
**TEST B **--Running packaged build with Administrator privileges
Both test paths behaved exactly as designed.
Consideration-
Based on the timing results above (~1 second for admin purge, ~80ms for fallback),
I'd love your recommendation for the GUI implementation:
1.simple toast message?
2.small modal?
3.mini-progress bar?
4.indeterminate spinner?
Happy to adjust based on your preference.