common, examples, llama : optimize using reserve if possible #5535
Conversation
examples/perplexity/perplexity.cpp
Outdated
| } | ||
|
|
||
| eval_pairs.clear(); | ||
| eval_pairs.reserve((i1 - i0)); |
There was a problem hiding this comment.
Similar issues to the above loop - best to just not reserve anything here.
common/train.cpp
Outdated
| out_samples_begin.clear(); | ||
| size_t end = (out_tokens.size() >= context_length) ? (out_tokens.size() - context_length) : 0; | ||
| out_samples_begin.reserve(end); | ||
| out_samples_begin.push_back(0); | ||
| out_samples_size.reserve(end); | ||
| out_samples_size.push_back(std::min((size_t) context_length, out_tokens.size())); | ||
| size_t end = (out_tokens.size() >= context_length) ? (out_tokens.size() - context_length) : 0; | ||
| for (size_t sample_begin = 1; sample_begin < end; ++sample_begin) { | ||
| out_samples_begin.push_back(sample_begin); | ||
| out_samples_size.push_back(context_length); |
There was a problem hiding this comment.
@xaedes It looks like we are clearing out_samples_begin but not out_samples_size, is that a bug?
examples/lookup/lookup.cpp
Outdated
| const int startIdx = i + ngram_size; | ||
| const int endIdx = startIdx + n_draft; | ||
| if (endIdx < inp_size) { | ||
| draft.reserve(endIdx - startIdx); |
There was a problem hiding this comment.
I don't think draft is necessarily empty here:
| draft.reserve(endIdx - startIdx); | |
| draft.reserve(draft.size() + endIdx - startIdx); |
| eval_pairs.clear(); | ||
| eval_pairs.reserve((i1 - i0) * 4); | ||
| for (size_t i = i0; i < i1; ++i) { | ||
| auto & hs_cur = hs_data[i]; | ||
| size_t li = hs_cur.common_prefix; | ||
| for (int s = 0; s < 4; ++s) { | ||
| eval_pairs.reserve((hs_cur.seq_tokens[s].size() - 1) - hs_cur.common_prefix); | ||
| for (size_t j = hs_cur.common_prefix; j < hs_cur.seq_tokens[s].size() - 1; j++) { | ||
| eval_pairs.emplace_back(hs_cur.i_batch + li++, hs_cur.seq_tokens[s][j + 1]); | ||
| } |
There was a problem hiding this comment.
These reserves of eval_pairs aren't right - reserving in a loop without accounting for the existing capacity is wrong, and the initial reserve is trying to be precise but misses the innermost loop.
examples/perplexity/perplexity.cpp
Outdated
| // Compute log-probs in parallel | ||
| // First we collect all tasks | ||
| eval_pairs.clear(); | ||
| eval_pairs.reserve(i1 - i0); |
llama.cpp
Outdated
| int index = 0; | ||
| size_t offset = 0; | ||
|
|
||
| symbols.reserve(word.size()); |
llama.cpp
Outdated
| const auto token = vocab.token_to_id.find(str); | ||
|
|
||
| if (token == vocab.token_to_id.end()) { | ||
| output.reserve(str.end() - str.begin()); |
There was a problem hiding this comment.
This isn't quite right - output isn't necessarily empty, we're just appending to it. And reserving in a loop is not going to do the right thing - best to just remove this.
llama.cpp
Outdated
| } | ||
| } | ||
|
|
||
| bpe_encoded_words.reserve(bpe_words.size()); |
There was a problem hiding this comment.
There is already a reserve() at the beginning of this function.
llama.cpp
Outdated
| size_t in_buff_offs = 0; | ||
| size_t out_buff_offs = 0; | ||
|
|
||
| workers.reserve(nthread); |
There was a problem hiding this comment.
There is already a reserve() of workers right when it is defined in the caller - reserving it here is pointless.
llama.cpp
Outdated
| first_row * n_per_row, this_nrow, n_per_row, local_hist.data(), imatrix); | ||
| } | ||
| }; | ||
| workers.reserve(nthread_use - 1); |
There was a problem hiding this comment.
There is already a reserve() of workers right when it is defined - reserving it here is pointless.
github-actions
bot
added
the
ggml
|
accidentally I use force push to an existing branch with PR that I forgot to finish. |
2 participants
usual use reserve function is where we know count elements