WIP: Faster io.unalloc for sequential reads ##io by trufae · Pull Request #25180 · radareorg/radare2

trufae · 2026-01-05T11:22:48Z

as discussed with @condret this is just a draft poc for inspiration. But i have some altrrnative ideas less intrussive to implement this

Copilot

Pull request overview

This PR implements a range-based validation cache to optimize performance of r_io_is_valid_offset for sequential read operations. The cache stores validation results for address ranges to avoid redundant validation checks during consecutive address queries.

Key changes:

Adds a new caching layer for offset validation that caches 1KB ranges around queried addresses
Implements cache initialization, cleanup, and invalidation functions
Integrates cache lookup into r_io_is_valid_offset to check cached results before performing actual validation

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 13 comments.

File	Description
libr/include/r_io.h	Defines new cache data structures (RIOVALIDRangeCache, RIOVALIDCache) and declares cache management API functions
libr/io/ioutils.c	Implements the validation cache with lookup, add, initialization, cleanup, and invalidation logic; integrates cache into r_io_is_valid_offset
libr/io/io.c	Adds cache initialization on RIO init and cleanup on RIO finalization
libr/io/io_map.c	Adds cache invalidation calls when maps are added or deleted to maintain cache consistency

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-05T11:29:56Z

+static bool r_io_valid_cache_lookup(RIO *io, ut64 addr, bool *result) {
+	RIOVALIDCache *cache = &io->valid_cache;
+
+	// Fast path: same as last query (common for consecutive bytes)
+	if (cache->last_query_addr != UT64_MAX && addr == cache->last_query_addr) {
+		*result = cache->last_query_result;
+		return true;
+	}
+
+	// Search through cached ranges
+	for (int i = 0; i < cache->count; i++) {
+		RIOVALIDRangeCache *range = &cache->ranges[i];
+		if (addr >= range->start_addr && addr <= range->end_addr) {
+			*result = range->is_valid;
+			cache->last_query_addr = addr;
+			cache->last_query_result = range->is_valid;
+			return true;
+		}
+	}
+	return false;
+}


The cache lookup doesn't check if the cache is properly initialized (ranges is NULL). If initialization failed or cache was freed, this will result in dereferencing a NULL pointer when accessing cache->ranges. Add a check for cache->ranges != NULL before attempting to access the cache.

Copilot · 2026-01-05T11:29:57Z

+static void r_io_valid_cache_add(RIO *io, ut64 addr, bool result) {
+	RIOVALIDCache *cache = &io->valid_cache;
+
+	// If cache is full, replace oldest entry (simple FIFO)
+	if (cache->count >= cache->capacity) {
+		// Shift all entries to make space at the end
+		if (cache->count > 0) {
+			memmove (&cache->ranges[0], &cache->ranges[1],
+				sizeof (RIOVALIDRangeCache) * (cache->count - 1));
+		}
+		cache->count--;
+	}
+
+	// Add new entry - we cache a small range around the address since
+	// most accesses are consecutive
+	RIOVALIDRangeCache *range = &cache->ranges[cache->count];
+	range->start_addr = addr;
+	range->end_addr = addr + 1023;  // Cache 1K range
+	range->is_valid = result;
+	// TODO: Set proper map_id when we can track it
+
+	cache->count++;
+	cache->last_query_addr = addr;
+	cache->last_query_result = result;
+}


The cache add function doesn't check if the cache is properly initialized (ranges is NULL). If initialization failed, this will attempt to write to NULL pointer. Add a check for cache->ranges != NULL before attempting to add entries.

Copilot · 2026-01-05T11:29:57Z

+	// If cache is full, replace oldest entry (simple FIFO)
+	if (cache->count >= cache->capacity) {
+		// Shift all entries to make space at the end
+		if (cache->count > 0) {
+			memmove (&cache->ranges[0], &cache->ranges[1],
+				sizeof (RIOVALIDRangeCache) * (cache->count - 1));
+		}
+		cache->count--;


The FIFO eviction strategy is inefficient for the stated use case of sequential reads. The memmove operation shifts all entries on every eviction, which is O(n) complexity. For sequential access patterns, an LRU or circular buffer would be more appropriate. Consider using a circular buffer with a head pointer to avoid the memmove overhead.

Copilot · 2026-01-05T11:29:57Z

+	range->start_addr = addr;
+	range->end_addr = addr + 1023;  // Cache 1K range
+	range->is_valid = result;
+	// TODO: Set proper map_id when we can track it


The comment suggests that map_id will be used for cache invalidation tracking, but the field is never set in this implementation. This leaves the field uninitialized (potentially containing garbage values from memory). Either implement the map_id tracking or remove the field and comment if it's not needed yet.

Suggested change

// TODO: Set proper map_id when we can track it

// map_id tracking is not implemented yet; use 0 as the default/sentinel.

range->map_id = 0;

Copilot · 2026-01-05T11:29:58Z

+}
+
+R_API void r_io_valid_cache_invalidate(RIO *io) {
+	R_RETURN_IF_FAIL (io);


The cache invalidation doesn't check if the cache is initialized (ranges is NULL). If initialization failed or the cache was not initialized yet, this will access uninitialized memory when setting last_query_addr. Add a check for io->valid_cache.ranges != NULL before accessing cache fields.

Suggested change

R_RETURN_IF_FAIL (io);

R_RETURN_IF_FAIL (io);

if (!io->valid_cache.ranges) {

return;

}

Copilot · 2026-01-05T11:30:00Z

+	for (int i = 0; i < cache->count; i++) {
+		RIOVALIDRangeCache *range = &cache->ranges[i];
+		if (addr >= range->start_addr && addr <= range->end_addr) {
+			*result = range->is_valid;
+			cache->last_query_addr = addr;
+			cache->last_query_result = range->is_valid;
+			return true;
+		}
+	}


The cache lookup uses a linear search through all cached ranges, which is O(n) where n is the number of cached entries (up to 64). For better performance with sequential reads, consider organizing ranges in a way that makes lookups more efficient, such as keeping them sorted and using binary search, or using the most recently used entry as a hint.

Copilot · 2026-01-05T11:30:00Z

 	if ((io->cache.mode & R_PERM_X) == R_PERM_X) {
 		// io.cache must be set to true for this codeblock to be executed
 		ut8 word[4] = { 0xff, 0xff, 0xff, 0xff};
 		// TODO: check for (io->cache.mode & R_PERM_S) ?
 		(void)r_io_read_at (io, offset, (ut8*)&word, 4);
 		if (!r_io_cache_read_at (io, offset, (ut8*)&word, 4)) {
 			if (!r_io_read_at (io, offset, (ut8*)&word, 4)) {
-				return false;
+				result = false;
 			}
 		}
-		return memcmp (word, "\xff\xff\xff\xff", 4) != 0;
-	}
-	if (io->mask) {
+		result = memcmp (word, "\xff\xff\xff\xff", 4) != 0;


Control flow issue: when the cache lookup path is taken (lines 104-114), the variable 'result' is set but may be overwritten by the memcmp on line 114 regardless of the read success. If both cache reads fail (line 110-112), result is set to false, but then line 114 unconditionally overwrites it with the memcmp result, which checks uninitialized or 0xff-filled memory. The 'result' assignment on line 111 should be followed by else clause wrapping line 114, or line 114 should only execute if reads succeed.

Copilot · 2026-01-05T11:30:00Z

 R_API bool r_io_is_valid_offset(RIO* io, ut64 offset, int hasperm) {
 	R_RETURN_VAL_IF_FAIL (io, false);
+
+	// Try cache lookup first for performance optimization
+	bool cached_result;
+	if (r_io_valid_cache_lookup (io, offset, &cached_result)) {
+		return cached_result;
+	}
+
+	// Compute actual result
+	bool result = false;
 	if ((io->cache.mode & R_PERM_X) == R_PERM_X) {
 		// io.cache must be set to true for this codeblock to be executed
 		ut8 word[4] = { 0xff, 0xff, 0xff, 0xff};
 		// TODO: check for (io->cache.mode & R_PERM_S) ?
 		(void)r_io_read_at (io, offset, (ut8*)&word, 4);
 		if (!r_io_cache_read_at (io, offset, (ut8*)&word, 4)) {
 			if (!r_io_read_at (io, offset, (ut8*)&word, 4)) {
-				return false;
+				result = false;
 			}
 		}
-		return memcmp (word, "\xff\xff\xff\xff", 4) != 0;
-	}
-	if (io->mask) {
+		result = memcmp (word, "\xff\xff\xff\xff", 4) != 0;
+	} else if (io->mask) {
 		if (offset > io->mask && hasperm & R_PERM_X) {
-			return false;
+			result = false;
+		} else {
+			goto check_permissions;
 		}
-	}
-	if (io->va) {
-		if (!hasperm) {
-			// return r_io_map_is_mapped (io, offset);
-			RIOMap* map = r_io_map_get_at (io, offset);
-			return map? map->perm & R_PERM_R: false;
+	} else {
+check_permissions:
+		if (io->va) {
+			if (!hasperm) {
+				// return r_io_map_is_mapped (io, offset);
+				RIOMap* map = r_io_map_get_at (io, offset);
+				result = map ? (map->perm & R_PERM_R) : false;
+			} else {
+				RIOMap* map = r_io_map_get_at (io, offset);
+				result = map ? ((map->perm & hasperm) == hasperm) : false;
+			}
+		} else {
+			if (!io->desc) {
+				result = false;
+			} else if (offset > r_io_desc_size (io->desc)) {
+				result = false;
+			} else {
+				result = ((io->desc->perm & hasperm) == hasperm);
+			}
 		}
-		RIOMap* map = r_io_map_get_at (io, offset);
-		return map? (map->perm & hasperm) == hasperm: false;
-	}
-	if (!io->desc) {
-		return false;
-	}
-	if (offset > r_io_desc_size (io->desc)) {
-		return false;
 	}
-	return ((io->desc->perm & hasperm) == hasperm);
+
+	// Cache the result for future use
+	r_io_valid_cache_add (io, offset, result);
+	return result;
 }


The cache is not invalidated on write operations. If the cache mode uses R_PERM_X for validation (line 104), writes to memory could change the validation status (e.g., writing 0xffffffff to make an address invalid, or writing other values to make it valid). The cache should be invalidated after write operations to ensure correctness.

Copilot · 2026-01-05T11:30:00Z

+R_API void r_io_valid_cache_init(RIO *io) {
+	R_RETURN_IF_FAIL (io);
+	io->valid_cache.ranges = calloc (VALID_CACHE_CAPACITY, sizeof (RIOVALIDRangeCache));
+	if (io->valid_cache.ranges) {
+		io->valid_cache.capacity = VALID_CACHE_CAPACITY;
+		io->valid_cache.count = 0;
+		io->valid_cache.last_query_addr = UT64_MAX;
+	}
+}
+
+R_API void r_io_valid_cache_fini(RIO *io) {
+	R_RETURN_IF_FAIL (io);
+	if (io->valid_cache.ranges) {
+		free (io->valid_cache.ranges);
+		memset (&io->valid_cache, 0, sizeof (io->valid_cache));
+	}
+}
+
+R_API void r_io_valid_cache_invalidate(RIO *io) {
+	R_RETURN_IF_FAIL (io);
+	io->valid_cache.count = 0;
+	io->valid_cache.last_query_addr = UT64_MAX;
+}
+
+// Check if address is in cached range
+static bool r_io_valid_cache_lookup(RIO *io, ut64 addr, bool *result) {
+	RIOVALIDCache *cache = &io->valid_cache;
+
+	// Fast path: same as last query (common for consecutive bytes)
+	if (cache->last_query_addr != UT64_MAX && addr == cache->last_query_addr) {
+		*result = cache->last_query_result;
+		return true;
+	}
+
+	// Search through cached ranges
+	for (int i = 0; i < cache->count; i++) {
+		RIOVALIDRangeCache *range = &cache->ranges[i];
+		if (addr >= range->start_addr && addr <= range->end_addr) {
+			*result = range->is_valid;
+			cache->last_query_addr = addr;
+			cache->last_query_result = range->is_valid;
+			return true;
+		}
+	}
+	return false;
+}
+
+// Add a validation result to cache
+static void r_io_valid_cache_add(RIO *io, ut64 addr, bool result) {
+	RIOVALIDCache *cache = &io->valid_cache;
+
+	// If cache is full, replace oldest entry (simple FIFO)
+	if (cache->count >= cache->capacity) {
+		// Shift all entries to make space at the end
+		if (cache->count > 0) {
+			memmove (&cache->ranges[0], &cache->ranges[1],
+				sizeof (RIOVALIDRangeCache) * (cache->count - 1));
+		}
+		cache->count--;
+	}
+
+	// Add new entry - we cache a small range around the address since
+	// most accesses are consecutive
+	RIOVALIDRangeCache *range = &cache->ranges[cache->count];
+	range->start_addr = addr;
+	range->end_addr = addr + 1023;  // Cache 1K range
+	range->is_valid = result;
+	// TODO: Set proper map_id when we can track it
+
+	cache->count++;
+	cache->last_query_addr = addr;
+	cache->last_query_result = result;
+}


The cache implementation is not thread-safe. Multiple threads can simultaneously read and modify the cache state (count, last_query_addr, last_query_result, and ranges array), leading to race conditions. If r_io_is_valid_offset can be called from multiple threads, add synchronization (e.g., mutex) to protect cache access.

Copilot · 2026-01-05T11:30:01Z

+	// Add new entry - we cache a small range around the address since
+	// most accesses are consecutive
+	RIOVALIDRangeCache *range = &cache->ranges[cache->count];
+	range->start_addr = addr;
+	range->end_addr = addr + 1023;  // Cache 1K range


The cached range assumes a 1KB (1024-byte) range is uniformly valid/invalid starting from the queried address. However, this assumption may be incorrect when:

The queried address is near a map boundary

Multiple maps with different validity states exist within this range

The address is near the end of a descriptor

This can lead to incorrect cache hits returning stale results. The cache should either validate range boundaries against actual map extents, or use a much smaller range (e.g., single byte) to avoid crossing boundaries.

Suggested change

// Add new entry - we cache a small range around the address since

// most accesses are consecutive

RIOVALIDRangeCache *range = &cache->ranges[cache->count];

range->start_addr = addr;

range->end_addr = addr + 1023; // Cache 1K range

// Add new entry - we cache the exact address

RIOVALIDRangeCache *range = &cache->ranges[cache->count];

range->start_addr = addr;

range->end_addr = addr; // Cache single-byte range to avoid crossing map boundaries

Faster io.unalloc for sequential reads ##io

375d6ba

Copilot AI review requested due to automatic review settings January 5, 2026 11:22

Copilot started reviewing on behalf of trufae January 5, 2026 11:24 View session

Copilot AI reviewed Jan 5, 2026

View reviewed changes

trufae changed the title ~~Faster io.unalloc for sequential reads ##io~~ WIP: Faster io.unalloc for sequential reads ##io Jan 5, 2026

trufae closed this Jan 9, 2026

trufae deleted the fast-unalloc branch January 14, 2026 10:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

WIP: Faster io.unalloc for sequential reads ##io#25180

WIP: Faster io.unalloc for sequential reads ##io#25180
trufae wants to merge 1 commit intomasterfrom
fast-unalloc

trufae commented Jan 5, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 5, 2026

Uh oh!

Copilot AI Jan 5, 2026

Uh oh!

Copilot AI Jan 5, 2026

Uh oh!

Copilot AI Jan 5, 2026

Uh oh!

Copilot AI Jan 5, 2026

Uh oh!

Copilot AI Jan 5, 2026

Uh oh!

Copilot AI Jan 5, 2026

Uh oh!

Copilot AI Jan 5, 2026

Uh oh!

Copilot AI Jan 5, 2026

Uh oh!

Copilot AI Jan 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	// TODO: Set proper map_id when we can track it
	// map_id tracking is not implemented yet; use 0 as the default/sentinel.
	range->map_id = 0;

Uh oh!

Conversation

trufae commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

trufae commented Jan 5, 2026 •

edited

Loading