In D990#18912, @mdawson wrote:

LGTM.

Out of curiosity, why is the QHash detaching for you here? Or is this just a general fixup found with a linter? Either way, I'm happy to take it :)

Well, we're calling QHash::find on a non-const variable -> QHash::find detaches internally. Showed up in a Callgrind run.

In D990#18917, @kfunk wrote:

In D990#18912, @mdawson wrote:

LGTM.

Out of curiosity, why is the QHash detaching for you here? Or is this just a general fixup found with a linter? Either way, I'm happy to take it :)

Well, we're calling QHash::find on a non-const variable -> QHash::find detaches internally. Showed up in a Callgrind run.

Right, but looking at this function it looks like the QHash shouldn't have multiple copies which should prevent a detach in the first place. Thus I was curious what work load triggered the copies causing the detach.

In D990#18925, @mdawson wrote:

In D990#18917, @kfunk wrote:

In D990#18912, @mdawson wrote:

LGTM.

Out of curiosity, why is the QHash detaching for you here? Or is this just a general fixup found with a linter? Either way, I'm happy to take it :)

Well, we're calling QHash::find on a non-const variable -> QHash::find detaches internally. Showed up in a Callgrind run.

Right, but looking at this function it looks like the QHash shouldn't have multiple copies which should prevent a detach in the first place. Thus I was curious what work load triggered the copies causing the detach.

Indeed, you're right. I just noticed it calls QHash::detach, but doesn't reallocate (-> non-shared container). This patch won't harm, though: constFind/constEnd is still faster, doesn't check whether the container is shared to begin with.

In D990#18933, @kfunk wrote:

In D990#18925, @mdawson wrote:

In D990#18917, @kfunk wrote:

In D990#18912, @mdawson wrote:

LGTM.

Out of curiosity, why is the QHash detaching for you here? Or is this just a general fixup found with a linter? Either way, I'm happy to take it :)

Well, we're calling QHash::find on a non-const variable -> QHash::find detaches internally. Showed up in a Callgrind run.

Right, but looking at this function it looks like the QHash shouldn't have multiple copies which should prevent a detach in the first place. Thus I was curious what work load triggered the copies causing the detach.

Indeed, you're right. I just noticed it calls QHash::detach, but doesn't reallocate (-> non-shared container). This patch won't harm, though: constFind/constEnd is still faster, doesn't check whether the container is shared to begin with.

Ah, that makes a lot more sense. I'm still happy with the patch, thus why I hit accept. I was just curious how it came about :)