Skip no-op visibility writes for unchanged samples · f60f09a

Skip no-op visibility writes for unchanged samples
Browse files

bogdan committed 5 days ago

main

1 parent c87dd3e

commit f60f09a

Showing 2 changed files with 27 additions and 28 deletions

+4 -2

HealthProbe/Doc/04-project/Import-Optimization-Log.md

@@ -314,6 +314,7 @@ from post-import UI recovery.
 | 2026-06-02 | `3dd5f48` | Fortified scheduled test database reset with a disk marker and extra SQLite sidecar cleanup. | Real-device report confirmed reset produced `previousSnapshotID: none`, `isChainStart: true`, and a clean first-snapshot timeline. |
 | 2026-06-02 | `06ee6be` | Removed unused `sample_versions(start_date, end_date)` and redundant `sample_visibility_ranges(sample_id, last_observation_id)` indexes. | Comparable first-import report was flat: wall clock 12m43s -> 12m39s and summed insert 10m11s -> 10m07s. Treat as no material performance change. |
 | 2026-06-02 | pending | Moved Dashboard archive cache refresh/rebuild off the UI path after snapshot completion. | Awaiting real-device confirmation that the app no longer stays unresponsive for roughly one minute after a completed snapshot. |
+| 2026-06-03 | pending | Fast-path unchanged samples whose current version already has an open visibility range. | Awaiting non-chain-start/full-scan report. Expected signal is lower insert elapsed on repeated captures with mostly unchanged rows, especially Heart Rate. |
 
 ## Current Diagnosis
 
@@ -329,6 +330,7 @@ The likely bottleneck is per-row SQLite work:
 - index maintenance while importing high-volume rows;
 - multiple dependent writes per sample;
 - commit / transaction shape;
+- no-op visibility range maintenance for unchanged existing samples;
 - Core Data or UI refresh work after SQLite completes. This became the active
   bottleneck after a no-delta incremental snapshot completed in 9.2s but the UI
   remained unresponsive for roughly one minute.
@@ -347,8 +349,8 @@ The likely bottleneck is per-row SQLite work:
 Prioritize experiments in this order:
 
 1. Confirm whether the background dashboard cache refresh removes the post-import UI freeze. If not, add explicit timings around cache rebuild, dashboard refresh, diagnostic report generation, and sheet dismissal.
-2. Run a non-chain-start/full-scan benchmark after skipping unchanged `verified` events and compare `SummedInsertElapsed`, `Heart Rate insertElapsed`, `Steps insertElapsed`, and `Walking + Running Distance insertElapsed`.
-3. Reduce remaining per-sample SQLite writes for unchanged existing samples during non-chain-start full scans, especially open visibility-range existence checks.
+2. Run a non-chain-start/full-scan benchmark after skipping unchanged `verified` events and fast-pathing already-open visibility ranges. Compare `SummedInsertElapsed`, `Heart Rate insertElapsed`, `Steps insertElapsed`, and `Walking + Running Distance insertElapsed`.
+3. Reduce any remaining per-sample SQLite writes for unchanged existing samples during non-chain-start full scans.
 4. Profile whether index maintenance dominates first-import insert cost.
 5. Consider a guarded bulk-import mode for first observations:
    - keep archive semantics unchanged;


+23 -26

HealthProbe/Services/SQLiteHealthArchiveStore.swift

View

@@ -2170,22 +2170,29 @@ actor SQLiteHealthArchiveStore: HealthArchiveStore {
                 statementCache: statementCache
             )
         } else {
-            try closeOpenVisibilityRanges(
-                sampleID: sampleResult.id,
-                excludingVersionID: versionResult.id,
-                closedAtObservationID: observationID,
-                observedAt: row.observedAt,
-                db: db,
-                statementCache: statementCache
-            )
-            try insertOpenVisibilityRangeIfNeeded(
+            if try !hasOpenVisibilityRange(
                 sampleID: sampleResult.id,
                 versionID: versionResult.id,
-                observationID: observationID,
-                observedAt: row.observedAt,
                 db: db,
                 statementCache: statementCache
-            )
+            ) {
+                try closeOpenVisibilityRanges(
+                    sampleID: sampleResult.id,
+                    excludingVersionID: versionResult.id,
+                    closedAtObservationID: observationID,
+                    observedAt: row.observedAt,
+                    db: db,
+                    statementCache: statementCache
+                )
+                try insertOpenVisibilityRange(
+                    sampleID: sampleResult.id,
+                    versionID: versionResult.id,
+                    observationID: observationID,
+                    observedAt: row.observedAt,
+                    db: db,
+                    statementCache: statementCache
+                )
+            }
         }
 
         return ArchiveV2SampleWriteResult(
@@ -2866,17 +2873,15 @@ actor SQLiteHealthArchiveStore: HealthArchiveStore {
         }
     }
 
-    private func insertOpenVisibilityRangeIfNeeded(
+    private func hasOpenVisibilityRange(
         sampleID: Int64,
         versionID: Int64,
-        observationID: Int64,
-        observedAt: Date,
         db: OpaquePointer?,
         statementCache: SQLiteStatementCache? = nil
-    ) throws {
+    ) throws -> Bool {
         let existing = try optionalInt64(
             """
-            SELECT first_observation_id
+            SELECT 1
             FROM sample_visibility_ranges
             WHERE sample_id = ? AND version_id = ? AND last_observation_id IS NULL
             LIMIT 1
@@ -2887,15 +2892,7 @@ actor SQLiteHealthArchiveStore: HealthArchiveStore {
             bindInt64(sampleID, to: 1, in: statement)
             bindInt64(versionID, to: 2, in: statement)
         }
-        guard existing == nil else { return }
-        try insertOpenVisibilityRange(
-            sampleID: sampleID,
-            versionID: versionID,
-            observationID: observationID,
-            observedAt: observedAt,
-            db: db,
-            statementCache: statementCache
-        )
+        return existing != nil
     }
 
     private func insertOpenVisibilityRange(


	@@ -314,6 +314,7 @@ from post-import UI recovery.
314	314	\| 2026-06-02 \| `3dd5f48` \| Fortified scheduled test database reset with a disk marker and extra SQLite sidecar cleanup. \| Real-device report confirmed reset produced `previousSnapshotID: none`, `isChainStart: true`, and a clean first-snapshot timeline. \|
315	315	\| 2026-06-02 \| `06ee6be` \| Removed unused `sample_versions(start_date, end_date)` and redundant `sample_visibility_ranges(sample_id, last_observation_id)` indexes. \| Comparable first-import report was flat: wall clock 12m43s -> 12m39s and summed insert 10m11s -> 10m07s. Treat as no material performance change. \|
316	316	\| 2026-06-02 \| pending \| Moved Dashboard archive cache refresh/rebuild off the UI path after snapshot completion. \| Awaiting real-device confirmation that the app no longer stays unresponsive for roughly one minute after a completed snapshot. \|
	317	+\| 2026-06-03 \| pending \| Fast-path unchanged samples whose current version already has an open visibility range. \| Awaiting non-chain-start/full-scan report. Expected signal is lower insert elapsed on repeated captures with mostly unchanged rows, especially Heart Rate. \|
317	318
318	319	## Current Diagnosis
319	320
	@@ -329,6 +330,7 @@ The likely bottleneck is per-row SQLite work:
329	330	- index maintenance while importing high-volume rows;
330	331	- multiple dependent writes per sample;
331	332	- commit / transaction shape;
	333	+- no-op visibility range maintenance for unchanged existing samples;
332	334	- Core Data or UI refresh work after SQLite completes. This became the active
333	335	bottleneck after a no-delta incremental snapshot completed in 9.2s but the UI
334	336	remained unresponsive for roughly one minute.
	@@ -347,8 +349,8 @@ The likely bottleneck is per-row SQLite work:
347	349	Prioritize experiments in this order:
348	350
349	351	1. Confirm whether the background dashboard cache refresh removes the post-import UI freeze. If not, add explicit timings around cache rebuild, dashboard refresh, diagnostic report generation, and sheet dismissal.
350		-2. Run a non-chain-start/full-scan benchmark after skipping unchanged `verified` events and compare `SummedInsertElapsed`, `Heart Rate insertElapsed`, `Steps insertElapsed`, and `Walking + Running Distance insertElapsed`.
351		-3. Reduce remaining per-sample SQLite writes for unchanged existing samples during non-chain-start full scans, especially open visibility-range existence checks.
	352	+2. Run a non-chain-start/full-scan benchmark after skipping unchanged `verified` events and fast-pathing already-open visibility ranges. Compare `SummedInsertElapsed`, `Heart Rate insertElapsed`, `Steps insertElapsed`, and `Walking + Running Distance insertElapsed`.
	353	+3. Reduce any remaining per-sample SQLite writes for unchanged existing samples during non-chain-start full scans.
352	354	4. Profile whether index maintenance dominates first-import insert cost.
353	355	5. Consider a guarded bulk-import mode for first observations:
354	356	- keep archive semantics unchanged;

	@@ -2170,22 +2170,29 @@ actor SQLiteHealthArchiveStore: HealthArchiveStore {
2170	2170	statementCache: statementCache
2171	2171	)
2172	2172	} else {
2173		- try closeOpenVisibilityRanges(
2174		- sampleID: sampleResult.id,
2175		- excludingVersionID: versionResult.id,
2176		- closedAtObservationID: observationID,
2177		- observedAt: row.observedAt,
2178		- db: db,
2179		- statementCache: statementCache
2180		- )
2181		- try insertOpenVisibilityRangeIfNeeded(
	2173	+ if try !hasOpenVisibilityRange(
2182	2174	sampleID: sampleResult.id,
2183	2175	versionID: versionResult.id,
2184		- observationID: observationID,
2185		- observedAt: row.observedAt,
2186	2176	db: db,
2187	2177	statementCache: statementCache
2188		- )
	2178	+ ) {
	2179	+ try closeOpenVisibilityRanges(
	2180	+ sampleID: sampleResult.id,
	2181	+ excludingVersionID: versionResult.id,
	2182	+ closedAtObservationID: observationID,
	2183	+ observedAt: row.observedAt,
	2184	+ db: db,
	2185	+ statementCache: statementCache
	2186	+ )
	2187	+ try insertOpenVisibilityRange(
	2188	+ sampleID: sampleResult.id,
	2189	+ versionID: versionResult.id,
	2190	+ observationID: observationID,
	2191	+ observedAt: row.observedAt,
	2192	+ db: db,
	2193	+ statementCache: statementCache
	2194	+ )
	2195	+ }
2189	2196	}
2190	2197
2191	2198	return ArchiveV2SampleWriteResult(
	@@ -2866,17 +2873,15 @@ actor SQLiteHealthArchiveStore: HealthArchiveStore {
2866	2873	}
2867	2874	}
2868	2875
2869		- private func insertOpenVisibilityRangeIfNeeded(
	2876	+ private func hasOpenVisibilityRange(
2870	2877	sampleID: Int64,
2871	2878	versionID: Int64,
2872		- observationID: Int64,
2873		- observedAt: Date,
2874	2879	db: OpaquePointer?,
2875	2880	statementCache: SQLiteStatementCache? = nil
2876		- ) throws {
	2881	+ ) throws -> Bool {
2877	2882	let existing = try optionalInt64(
2878	2883	"""
2879		- SELECT first_observation_id
	2884	+ SELECT 1
2880	2885	FROM sample_visibility_ranges
2881	2886	WHERE sample_id = ? AND version_id = ? AND last_observation_id IS NULL
2882	2887	LIMIT 1
	@@ -2887,15 +2892,7 @@ actor SQLiteHealthArchiveStore: HealthArchiveStore {
2887	2892	bindInt64(sampleID, to: 1, in: statement)
2888	2893	bindInt64(versionID, to: 2, in: statement)
2889	2894	}
2890		- guard existing == nil else { return }
2891		- try insertOpenVisibilityRange(
2892		- sampleID: sampleID,
2893		- versionID: versionID,
2894		- observationID: observationID,
2895		- observedAt: observedAt,
2896		- db: db,
2897		- statementCache: statementCache
2898		- )
	2895	+ return existing != nil
2899	2896	}
2900	2897
2901	2898	private func insertOpenVisibilityRange(