Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(196)

Issue 2579473002: [wip] Add LiteralStringPiece which templatizes based on character length (Closed)

Created:
4 years ago by Charlie Harrison
Modified:
3 years, 10 months ago
Reviewers:
Nico
CC:
chromium-reviews, jshin+watch_chromium.org, vmpstr+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add LiteralStringPiece which templatizes based on character length StringPiece is often constructed out of string literals, which entails a call to strlen() or equivalent. This is unnecessary, as we know the length of a literal at compile time. BUG=673376

Patch Set 1 #

Patch Set 2 : add some usage #

Unified diffs Side-by-side diffs Delta from patch set Stats (+20 lines, -7 lines) Patch
M base/strings/string_piece.h View 1 1 chunk +5 lines, -0 lines 0 comments Download
M base/strings/string_piece_unittest.cc View 1 chunk +7 lines, -0 lines 0 comments Download
M extensions/browser/api/web_request/web_request_permissions.cc View 1 2 chunks +8 lines, -7 lines 0 comments Download

Messages

Total messages: 12 (7 generated)
Charlie Harrison
Nico: are you a good reviewer for this? There are places in the codebase that ...
4 years ago (2016-12-14 21:57:02 UTC) #5
Nico
i'm super behind, so i won't get to this super soon at lesat. yes, this ...
4 years ago (2016-12-14 21:58:54 UTC) #6
Charlie Harrison
I tested two builds, one using the templatized function, and one removing the template param, ...
4 years ago (2016-12-14 23:14:53 UTC) #9
esprehn
The optimizer is actually removing all of these calls to strlen. We don't need this ...
3 years, 12 months ago (2016-12-25 21:40:11 UTC) #10
esprehn
3 years, 12 months ago (2016-12-25 22:05:46 UTC) #11
On 2016/12/25 at 21:40:11, esprehn wrote:
> The optimizer is actually removing all of these calls to strlen. We don't need
this literal class, I even removed the equivalent one in blink after verifying
the assembly output.

That is if you're seeing this on profiles I'd love to see it and the associated
callstacks. :)

Powered by Google App Engine
This is Rietveld 408576698