mem: Store snoop filter lookup result to avoid second lookup

Information
Submitter:	Andreas Hansson
Repository:	gem5
Branch:	default
Bugs:
Depends On:
Reviewers
Groups:	Default
People:

Description

Changeset 11062:85ac67b53235
---------------------------
mem: Store snoop filter lookup result to avoid second lookup

This patch introduces a private member storing the iterator from the
lookupRequest call, such that it can be re-used when the request
eventually finishes. The method previously called updateRequest is
renamed finishRequest to make it more clear that the two functions
must be called together.

Testing Done

Seems fine to me.  Just one question, is it necessary to keep

the lookup result as a class variable?  Can you not pass it 

around after the first lookup?  Some how, I do not like doing this.

Andreas Hansson Sept. 23, 2015, 10:23 a.m. (Sept. 23, 2015, 10:23 a.m.)

You are not the only one. We have had many heated debates, and the main argument for using the class variable is to not have to expose the types outside the snoop filter, and leave the responsibility of tracking very specific iterators to the crossbar. Not great...but it still seems like the best solution.

Nilay Vaish Sept. 24, 2015, 9:19 a.m. (Sept. 24, 2015, 9:19 a.m.)

Are you sure it is reliable to store the iterator? Typically any modification to the
underlying data structure invalidates them. en.cppreference.com tells me that insertion and emplace
to an unordered_map may invalidate all iterators.

Andreas Hansson Sept. 24, 2015, 9:22 a.m. (Sept. 24, 2015, 9:22 a.m.)

The second function needs to be called before any changes are made. It's not great...but I cannot think or any better way of doing it.

Nilay Vaish Sept. 24, 2015, 9:30 a.m. (Sept. 24, 2015, 9:30 a.m.)
```
Did you consider returning the iterator from lookupRequest()?
```

Nilay Vaish Sept. 24, 2015, 9:32 a.m. (Sept. 24, 2015, 9:32 a.m.)

OK, now I get your point on not exposing types to the crossbar.

Andreas Hansson Sept. 24, 2015, 9:32 a.m. (Sept. 24, 2015, 9:32 a.m.)

As I mentioned earlier, we have had many heated debates, and the main argument for using the class variable is to not have to expose the types outside the snoop filter, and leave the responsibility of tracking very specific iterators to the crossbar. Not great...but it still seems like the best solution. Otherwise we are just pushing the problem/responsibility to the crossbar.

Nilay Vaish Sept. 24, 2015, 9:35 a.m. (Sept. 24, 2015, 9:35 a.m.)

I am fine with this for now. You can go ahead and push all these snoop filter patches.

You have a pending review.

Review Board 2.0.15

This change has been marked as submitted.

Screenshots

Files

Status: Closed (submitted)