Search returning out-of-bounds results

[bryevdv]

When the viewport is zoomed into a smaller interval, and the search is invoked, then the returned results can include items outside the viewport, contrary to expectation. The root cause appears to be a missed cache invalidation for unexpanded results.

cc @lightsighter for a profile and search term to use to reproduce.

[lightsighter]

I've sent the data and the instructions offline since they are too big to upload here.

[elliottslaughter]

Just wanted to give one point of direction to @bryevdv. What I showed you on the call was the draw path. But I think invalidating the meta tiles is not the right approach there. I think the place to check is inflate_meta, which is called by the search path (and already handles all the logic for if we're searching over unexpanded processors). Basically, this code needs invalidation logic:

prof-viewer/src/app.rs

Lines 783 to 787 in 2f8dfcd

	fn inflate_meta(&mut self, config: &mut Config, cx: &mut Context) {
	for tile_id in config.request_tiles(cx.view_interval) {
	self.fetch_meta_tile(tile_id, config);
	}
	}

(If you follow up the call stack you'll get to the place where this is called in the search algorithm.)

[elliottslaughter]

I think we fixed this with #32?

[bryevdv]

@elliottslaughter I don't think so? #32 just added the whole-word search option, which makes it easier to pare down search for certain kinds of results, e.g. ids that have fences like <123>

[elliottslaughter]

You're right. The cache invalidation bug is still there. Sorry about the noise.

[bryevdv]

Hi @elliottslaughter @lightsighter I want to make sure I understand this bug.

• Starting all the way zoomed out, see an item with "71"

  

• Search for "71" (still zoomed out) see the result, all good:

  

• Reset the search, zoom into a region without the "71" then search again — no result shown, still all good?

  

  I had thought this was the reported error case, and expected to see a result show up even when searching in the zoomed-away area. But it seems to be working as I'd expect.

---

By contrast: If I start from the zoomed out view with a search result (bullet two above), and then zoom away, without resetting anything first, then the result stays in the search, which I would not expect:

Is this different from the original reported bug? Regardless, just to confirm: the search results should reset when the view interval changes?

[lightsighter]

Reset the search, zoom into a region without the "71" then search again — no result shown, still all good?

If I scroll right or left, will the results automatically refresh?

I had thought this was the reported error case, and expected to see a result show up even when searching in the zoomed-away area. But it seems to be working as I'd expect.

I honestly don't remember what I was trying to do that was behaving funny. I just remembered you guys agreed with me that it was strange when I showed it to you. 😇

By contrast: If I start from the zoomed out view with a search result (bullet two above), and then zoom away, without resetting anything first, then the result stays in the search, which I would not expect:

By "zoom away" do you mean "zoom in"? If so I think this might have been my case too. I was trying to refine a search and wanted to see results getting pruned out as the window size shrunk.

Regardless, just to confirm: the search results should reset when the view interval changes?

I think in general this in the invariant we want to maintain: the search results should always reflect what is visible. Anything that causes the viewport to change, should cause the search results to refresh.

[elliottslaughter]

I think the bug involved a combination of zooming + the "search unexpanded processors" option. Because those processors are unexpanded, the cache invalidation logic doesn't kick in and we get stale results. I think that's why I made my comment about inflate_meta here: #30 (comment)

Try this in a fresh profile:

1. Find the task you want to search for
2. Collapse all the processors (so you can't see it anymore)
3. Search (should be empty; this is correct)
4. Click "search unexpanded processors"
5. It should appear in search now (this is correct)
6. Zoom away (anywhere that the task isn't, so the task isn't in the view interval)
7. The task should still appear in the search (this is the bug)