On 09/26/2010 04:47 PM, Kinkie wrote:
*
* A SBuf can be empty, but can never be undefined. In other words
there is no equivalent
* of NULL. defined/undefined semantics need to be implemented by
the
caller.
*/
Remove: Avoid documenting what SBuf cannot be. If you insist on
discussing
the non-existent APIs, move that discussion to the SBuf.dox file.
How about moving a simplified version of it that to the class
documentation? I have committed such an attempt.
You are still documenting negatives, which is counter-productive. I know
what you mean by "can never be undefined" but a new developer will not know
how to interpret that because there is no defined() function in C/C++.
Is "A SBuf can be empty (zero-length), but always points to a valid
memory address." clearer?
Not really because from the user point of view SBuf does not point to
anything at all. There are no "*" or "->" operators for SBuf.
/** Assignment operation with printf(3)-style definition
* \note arguments may be evaluated more than once, be careful
* of side-effects
*/
SBuf& Printf(const char *fmt, ...);
Add "Avoid this and other ... methods. For transition only."?
We should be using ostream formatting instead.
Agreed, if only I could think of a sane way to do so (my ignorance).
If you point me to a specific (but hopefully rather common) use case where
you cannot use ostream, I will try to come up with a sane way of using it.
Most likely, it will require adding a FixedSizeStream class of sorts.
My ignorance is in a way to implement an ostream interface with a
custom backing store.
Amos has shed some light for me (StoreEntryStream), I'll need help or
some more detailed documentation to do it on my own.
AFAIK, you need a custom streambuf class that you can then use to create
an output stream that would write to your buffer. Attached file
implements both using (const char *buffer, size) pair as a fixed-size
buffer. The code is known to work in some environments, but may contain
bugs or missing features. I do not know whether it has been tested
outside of the GCC world.
You may import the provided (buffer, size) API as is. If you do, many
current users would benefit. You may change that API to use SBuf
instead. Only new users would benefit. Finally, you can leave the old
API, add SBuf-based API, and implement the old API using SBuf. That way,
both old and new users would benefit.
If you find any bugs, please let me know so that we can fix them in
other projects as well.
StoreEntryStreamBuf appears to do similar stuff but for a non-fixed
buffer. You should probably use that code as an example if you decide to
make a non-fixed SBuf-based stream. It is also likely that the attached
code can borrow some optimizations from StoreEntryStreamBuf.
As discussed on IRC, will move to
const char * rawContent() const
(-> raw access to memory for READING)
char * rawSpace()
(-> raw access to memory for WRITING, does cow(). Return a pointer to
the first unused byte in the backend store)
There is a problem with this approach though. If we write to the SBuf
outside its import methods, we also need to signal it how much we
wrote (e.g. a method "haveAppended"). But it's lots of layer-breaking
:(
Since we decided to provide write access to the raw buffer, the API has
to have a minimum set of methods, including appended(). There is nothing
wrong with that, IMO.
To summarize and polish both for const:
// not 0-terminated, writeable internal storage; AVOID
char *rawStorage();
// terminated c-string; may not be internal storage; SLOW
const char *c_str();
Eventually, we should make c_str() a const method but let's not go there
now. You may add a TODO about that.
Both are now const. Easier to make them non-const if needed than the opposite.
You did not make c_str() const as far as I can see (r9518). It is not
trivial to do so because c_str() calls terminate() which is not const.
Let's not do that for now.
u_int16_t nreallocs; /// number of reallocs this SBuf has been subject
to
Since nreallocs is copied on assignment, it's true meaning is rather
difficult to define. However, it is _not_ the number of reAlloc() calls
that _this_ SBuf has seen.
nreallocs is meant to be a hint to the memory management routines to
define the likeness of needing a grow operation, and thus to define
how much headroom to leave. It's currently unused in favour of a
static (20% headroom + rounding) approach.
Any feedback on optimizing the heuristic and when to propagate the
hint is welcome.
My feedback is to remove the unused member until somebody properly defines
and uses it.
I've tried to define it instead.
I do not see any changes in nrealloc description so it is still does not
reflect the reality. Please do not say that something is done until you
commit the corresponding change.
The algorithm needs tweaking, but
this should already
give an idea about what it may be about. The useage scenario it is
designed for is:
SBuf s;
while (s.append(something())) {
SBuf read=s.consume(chunk)
doSomethingWith(read);
}
s will get realloc'ed more and more; by giving more headroom we ensure
that it will adapt to the useage pattern.
SBuf::size_type SBuf::estimateCapacity (SBuf::size_type desired) const
{
if (nreallocs< 10)
return 2*desired;
if (nreallocs< 100)
return 4*desired;
if (nreallocs< 1000)
return 8*desired;
return 16*desired;
}
I understand what you are trying to do here, but I have no idea whether
such optimization would help or hurt. It would be nice to focus on
essential pieces first and then add questionable optimizations later,
after they have been tested and proved their effectiveness. It is often
much easier to add a good new optimization than to find and remove a bad
old one.
_SQUID_INLINE_ char * buf() const;
_SQUID_INLINE_ char * bufEnd() const;
Why const if they allow SBuf modification?
They don't allow modifications. They are private functions to get
pointers and end of "our" store, for interfacing with the legacy C
world.
They should return "const char *" if they do not allow modification.
I believe I wasn't clear: those are simply private mehtods meant to
help with recurring activities.
That does not answer my original question: Why const if they allow SBuf
modification? Either they should return "const char *" or they should
not be const themselves.
I will come back to SBuf once MemBlob is committed.
Cheers,
Alex.
/* (C) 2005 The Measurement Factory
Licensed under the Apache License, Version 2.0 */
#ifndef TMF_BASE_XSTD__FXSTREAM_H
#define TMF_BASE_XSTD__FXSTREAM_H
#if defined(XSTD_HAVE_STREAMBUF)
# include <streambuf>
#elsif defined(XSTD_HAVE_STREAMBUF_H)
# include <streambuf.h>
#endif
// XXX: Keep in sync with the same classes in Polygraph's src/xstd/h/sstream.h
// A streambuf with a fixed-size user-owned storage area.
// The implementation is incomplete and too forgiving,
// but hopefully sufficient.
class FixedStreamBuf: public std::streambuf {
public:
// based on http://www.dbforums.com/showthread.php?t=776452
FixedStreamBuf(char_type *buffer, std::streamsize size) {
setp(buffer, buffer + size);
setg(buffer, buffer, buffer + size);
}
// set position relative to dir
virtual pos_type seekoff(off_type off, std::ios_base::seekdir dir,
std::ios_base::openmode mode = std::ios_base::in | std::ios_base::out) {
pos_type result = pos_type(-1);
if (mode & std::ios_base::out) {
// convert to absolute offset if needed
if (dir == std::ios_base::cur)
off = pptr() - pbase() + off;
else
if (dir == std::ios_base::end)
off = epptr() - pbase() - off;
result = seekpos(off);
}
// XXX: support std::ios_base::in mode
return result;
}
// set absolute position (i.e., relative to the base)
virtual pos_type seekpos(pos_type off,
std::ios_base::openmode mode = std::ios_base::in | std::ios_base::out) {
pos_type result = pos_type(-1);
if (mode & std::ios_base::out) {
if (0 <= off && off <= (epptr() - pbase())) {
pbump(off - pos_type(pptr() - pbase()));
result = off;
}
}
// XXX: support std::ios_base::in mode
return result;
}
};
// an output stream that uses our FixedStreamBuf
class OFixedStream: public std::ostream {
public:
OFixedStream(char_type *buffer, std::streamsize size):
theBuffer(buffer, size) {
this->init(&theBuffer); // set our custom buffer
}
private:
FixedStreamBuf theBuffer;
};
// More "fixed size stream buffer" ideas at
// http://www.inf.uni-konstanz.de/~kuehl/c++/iostream/
// http://www.velocityreviews.com/forums/showpost.php?p=1532770
#endif
