libbpf-tools/profile: Batch lookup support for hash map #4990
Conversation
ekyooo
requested review from
drzaeus77,
goldshtn,
yonghong-song,
4ast,
brendangregg and
davemarchevsky
as code owners
libbpf-tools/profile.c
Outdated
| } else { | ||
| return err; | ||
| } | ||
| } |
There was a problem hiding this comment.
In map_helpers.c, we have dump_hash(). Could you try to use that function?
There was a problem hiding this comment.
Yes, there is duplicate code in this patch.
read_counts_map() in profile.c returns data as an array of type "struct key_ext_t", which is a combination of the keys and values from the hash map. This is the difference from the design of dump_hash() in map_helpers.c.
In the current patch, the combination is only needed for lookup_batch. However, using map_helpers' dump_hash() requires combination (data copying) for both the batch and iteration cases. Perhaps that's why syscount.c doesn't use map_helpers either. Could you tell me what you think about it?
There is also a way to utilize only dump_hash_batch() of map_helpers.c after removing static.
Thank you.
There was a problem hiding this comment.
I think dump_hash() should work. It will cover both batch and non-batch case. I suggest to replace non-batch case with dump_hash(). Could you give a try?
There was a problem hiding this comment.
Of course, I'll update it.
Use bpf_map_lookup_batch() instead of bpf_map_get_next_key() to read the hash map, if available.
|
|
||
| lookup_key = &items[i].k; | ||
| i++; | ||
| for (i = 0; i < n; i++) { |
There was a problem hiding this comment.
In previous patches, maps were read directly into the "items" variable, so this copy would not work unless it was in a batch. Now we use a little more memory and CPU. However, this change is good for eliminating duplicate code.
|
Could you let me know if I've misunderstood any of your comments? |
Use bpf_map_lookup_batch() instead of bpf_map_get_next_key() to read the hash map, if available.