Skip to content
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

msg/simple: apply prefetch policy more precisely #10344

Merged
merged 1 commit into from Aug 1, 2016

Conversation

xiexingguo
Copy link
Member

@xiexingguo xiexingguo commented Jul 19, 2016

We shall apply prefetch policy based on the residual length aftering
checking cache instead of the original request length.

E.g., if the reading sequences are 1K, 5K, 2K, the improved logic
will trigger another prefetch of 4K(as 5K - 3K(from recv_buf) == 2K, and we
now have 8K prefetched data total) by the second 5K reading(which we
don't do this according to the old logic), and thus the last reading request
which asks for 2K data can be also benefited from this prefetch,
which is good for performance.

Signed-off-by: xie xingguo xie.xingguo@zte.com.cn

We shall apply prefetch policy based on the residual length aftering
checking cache instead of the original request length.

E.g., if the reading sequences are 1K, 5K, 2K, the improved logic
will trigger another prefetch of 4K(as 5K - 3K(from recv_buf) == 2K, and we
now have 8K prefetched data total) by the second 5K reading(which we
don't do this according to the old logic), and thus the last reading request
which asks for 2K data can be also benefited from this prefetch too,
which is good for performance.

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
@gregsfortytwo
Copy link
Member

Doesn't this change contradict the comment right there about not prefetching large reads?

@xiexingguo
Copy link
Member Author

xiexingguo commented Jul 20, 2016

The key difference is how do we define large reads.
Previously we honour the origin length of read only when applying prefetch policy. But here we reconsider the residual length of read after reading from the cache instead.

E.g., suppose the recv_max_prefetch is 4K, and the read sequences are 1K, 5K, 2K

Before this change:

  1. read 1K, as 1K < recv_max_prefetch, we prefetch 4K, and the 1K read itself is hit in the cache after the prefetch is done.
  2. read 5K, the first 3K is hit in the cache and the cache is now empty, as 5K > recv_max_prefetch, we don't prefetch and trigger a 2K read instead.
  3. read 2K, the cache is now empty, as 2K > recv_max_prefetch, we trigger another prefetch and get 2K from the cache after prefetch is done.

After this change:

  1. read 1K, as 1K < recv_max_prefetch, we prefetch 4K, and the 1K read itself is hit in the cache after the prefetch is done.
  2. read 5K, the first 3K is hit in the cache and the cache is now empty and we have 5K-3K = 2K to read, as 2K < recv_max_prefetch, we prefetch again and get 2K from the cache after prefetch is done, the cache has 2K data remaining.
  3. read 2K, which is directly hit in the cache.

From the above example, we need exactly 2 (prefetch)reads now instead of 3 reads which we need before.

@xiexingguo
Copy link
Member Author

@gregsfortytwo Thoughts?

@gregsfortytwo
Copy link
Member

Yeah, that makes sense. I was worried about the performance impact of the change but going down a few function frames it all looks good to me!
Reviewed-by:

@yuriw
Copy link
Contributor

yuriw commented Aug 1, 2016

@jdurgin jdurgin merged commit d5c12af into ceph:master Aug 1, 2016
jdurgin added a commit that referenced this pull request Aug 1, 2016
msg/simple: apply prefetch policy more precisely

Reviewed-by: Greg Farnum <gfarnum@redhat.com>
Conflicts:
	src/msg/simple/Pipe.cc (removed unneeded cast)
@xiexingguo xiexingguo deleted the xxg-wip-pipe-2016-07-19-02 branch August 1, 2016 22:44
xiexingguo added a commit to xiexingguo/ceph that referenced this pull request Jan 3, 2019
We shall apply prefetch policy based on the residual length
instead of the original requested length.

E.g., suppose the recv_max_prefetch is 4K, and the read sequences are 1K, 5K, 2K

**Before this change:**
- read 1K, as 1K < recv_max_prefetch, we prefetch 4K, and the 1K read
  itself is hit in the cache after the prefetch is done.
- read 5K, the first 3K is hit in the cache and the cache is now empty,
  as 5K > recv_max_prefetch, we don't prefetch and trigger a 2K read instead.
- read 2K, the cache is now empty, as 2K > recv_max_prefetch, we trigger
  another prefetch and get 2K from the cache after prefetch is done.

**After this change:**
- read 1K, as 1K < recv_max_prefetch, we prefetch 4K, and the 1K read
  itself is hit in the cache after the prefetch is done.
- read 5K, the first 3K is hit in the cache and the cache is now empty
  and we have 5K-3K = 2K to read, as 2K < recv_max_prefetch, we prefetch
  again and get 2K from the cache after prefetch is done, the cache has
  2K data remaining.
- read 2K, which is directly hit in the cache.

From the above example, we need exactly 2 (prefetch)reads now instead of
3 reads which we need before.

See-also: ceph#10344
Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants