Comments (6)
I can confirm this. (MSVC 9.0 SP1, x64)
Original comment by [email protected]
on 8 May 2013 at 9:20
from snappy.
Forgot to add: Happens with Snappy 1.1.0
Original comment by [email protected]
on 8 May 2013 at 9:22
from snappy.
I've looked a bit of these.
Note that in general, Snappy is not meant to support compressing blocks of
64-bit size, so in itself, this is not a very high-priority goal.
We have a few classes of these warnings: One is the kind where we knowingly
narrow the size_t into something much smaller. See e.g. this example from
EmitCopyLessThan64, with len_minus_4 and offset being size_t, and op being
char*:
*op++ = COPY_1_BYTE_OFFSET + ((len_minus_4) << 2) + ((offset >> 8) << 5);
*op++ = offset & 0xff;
We already have ample asserts here, so there's no bug that the warning
uncovers. To fix it, we could have to do something like
*op++ = static_cast<uint8>(COPY_1_BYTE_OFFSET + ((len_minus_4) << 2) + ((offset >> 8) << 5));
*op++ = static_cast<uint8>(offset & 0xff);
except we'd need to break the line:
*op++ = static_cast<uint8>(COPY_1_BYTE_OFFSET + ((len_minus_4) << 2) +
((offset >> 8) << 5));
*op++ = static_cast<uint8>(offset & 0xff);
I think this reduces readability enough that it's a clear negative.
Another, more interesting class is in cases like this:
static inline void IncrementalCopyFastPath(const char* src, char* op, int len) {
where IncrementalCopyFastPath is called with a size_t argument for len. Also,
we repeatedly do things like:
len -= op - src;
We can't have len be size_t since we need to be able to check for it being
negative; however, it could reasonably be ssize_t. As an added bonus, it would
then seem we can avoid a signed 32-to-64-bit conversion on a hot path, which
wins us something like 2% in decompression. It looks like this varies a bit
between GCC versions, though; it's easy to get unlucky with what's presumably a
good change.
Unfortunately, just changing everything blindly to ssize_t where it used to be
int doesn't really help; seemingly, the cases here have to be implemented and
benchmarked on a case-by-case basis, since some of them appears to hurt
performance (for whatever reason).
So, in short, while some of these are interesting, I don't think we can fix all
of them without unduly hurting readability and/or speed, so in that case, I'd
say you should simply disable the warning in Chromium.
Comments?
Original comment by [email protected]
on 14 Jun 2013 at 2:52
from snappy.
Yep, we've already disabled the warning in Chromium. Thanks for taking a look.
Original comment by [email protected]
on 14 Jun 2013 at 5:02
from snappy.
r77 is relevant for this bug.
I'll leave it open until I get to look if the other cases have performance
impact, just so I have a tracker on that. After that, I'll close it, since
we're not going to be fixing all of them.
Original comment by [email protected]
on 14 Jun 2013 at 9:44
from snappy.
Found no further ones. Closing as mentioned.
Original comment by [email protected]
on 1 Jul 2013 at 11:03
- Changed state: WontFix
from snappy.
Related Issues (20)
- Does not handle data larger than 4GB HOT 8
- Mistakes on the start page HOT 5
- cppcheck - Member variable is not initialized in the constructor. HOT 2
- Type 'ssize_t' not defined for MSVC builds HOT 4
- snappy needs a command line utility HOT 5
- MIsspelled in code HOT 1
- testdata/mapreduce-osd-1-pdf contains "DO NOT DISTRIBUTE" disclaimer HOT 3
- Patch for compiling Snappy with MSVC on Windows HOT 3
- Bug in IncrementalCopyFastPath HOT 6
- use ctypes.util.find_library HOT 3
- ARMv6 and unaligned access HOT 3
- Decompression issues with Snappy 1.1.2 HOT 3
- ahsan ullah HOT 2
- Seeing Null Values from Hive with Snappy Compression HOT 1
- No versioned link for current build HOT 2
- ppc64le entry is needed in config.guess file HOT 4
- performance issue in snappy.cc - I am using version 1.1.2 HOT 2
- Unnecessary memory allocation in snappy.cc:Compress HOT 1
- bad_alloc exception not caught in snappy.cc::Compress
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from snappy.