Hi,
here's the updated patch (missing the Makefile.am bits, I'm
cherrypicking and it's just easier to do merge them by hand).
The merge will involve changesets 9679-9681, 9683,9687 of branch
lp:~squid/squid/stringng .
Thanks
On Sun, Nov 24, 2013 at 10:26 AM, Kinkie <gkin...@gmail.com> wrote:
>>>> in src/SBufDetailedStats.cc
>>>> * please name the namespace
>>>
>>> What is the benefit? The code in there is just in order to have the
>>> post-init stuff done in StatHist, it's not meant to be seen anywhere
>>> else (so the anonymous namespace is really intentional). Not a big
>>> deal, but I'm curious.
>>
>> It will save on getting doxygen confused. And if we ever need to
>> reference inside that scope or add bits to it the namespace name
>> provides a hint as to what might be appropriate use.
>
> Ok, fair enough. I will add a name to it.
> Once that's done, may I submit?
> (btw: that post-init code is the result of StatHist C-ness, it should
> go away, I'd vebture by subclassing and polymorphism)
>
> Thanks
>
> --
> /kinkie
--
/kinkie
=== modified file 'src/MemBlob.cc'
--- src/MemBlob.cc 2013-10-04 13:55:21 +0000
+++ src/MemBlob.cc 2013-11-22 22:24:51 +0000
@@ -31,6 +31,7 @@
#include "Debug.h"
#include "Mem.h"
#include "MemBlob.h"
+#include "SBufDetailedStats.h"
#if HAVE_IOSTREAM
#include <iostream>
@@ -96,6 +97,7 @@
memFreeString(capacity,mem);
Stats.liveBytes -= capacity;
--Stats.live;
+ recordMemBlobSizeAtDestruct(size);
debugs(MEMBLOB_DEBUGSECTION,9, HERE << "destructed, this="
<< static_cast<void*>(this) << " id=" << id
=== modified file 'src/SBuf.cc'
--- src/SBuf.cc 2013-10-07 11:23:58 +0000
+++ src/SBuf.cc 2013-11-22 22:24:51 +0000
@@ -31,6 +31,7 @@
#include "Debug.h"
#include "OutOfBoundsException.h"
#include "SBuf.h"
+#include "SBufDetailedStats.h"
#include "SBufExceptions.h"
#include "util.h"
@@ -148,6 +149,7 @@
{
debugs(24, 8, id << " destructed");
--stats.live;
+ recordSBufSizeAtDestruct(len_);
}
MemBlob::Pointer
=== added file 'src/SBufDetailedStats.cc'
--- src/SBufDetailedStats.cc 1970-01-01 00:00:00 +0000
+++ src/SBufDetailedStats.cc 2013-11-24 16:55:30 +0000
@@ -0,0 +1,47 @@
+#include "squid.h"
+#include "SBufDetailedStats.h"
+#include "StatHist.h"
+
+/*
+ * Implementation note: the purpose of this construct is to avoid adding
+ * external dependencies to the SBuf code
+ */
+
+static StatHist sbufDestructTimeStats;
+static StatHist memblobDestructTimeStats;
+
+namespace SBufDetailedStatsHistInitializer {
+ // run the post-instantiation initialization methods for StatHist objects
+ struct Initializer
+ {
+ Initializer() {
+ sbufDestructTimeStats.logInit(300,30.0,128000.0);
+ memblobDestructTimeStats.logInit(300,30.0,128000.0);
+ }
+ };
+ Initializer initializer;
+}
+
+void
+recordSBufSizeAtDestruct(SBuf::size_type sz)
+{
+ sbufDestructTimeStats.count(static_cast<double>(sz));
+}
+
+const StatHist *
+collectSBufDestructTimeStats()
+{
+ return &sbufDestructTimeStats;
+}
+
+void
+recordMemBlobSizeAtDestruct(SBuf::size_type sz)
+{
+ memblobDestructTimeStats.count(static_cast<double>(sz));
+}
+
+const StatHist *
+collectMemBlobDestructTimeStats()
+{
+ return &memblobDestructTimeStats;
+}
=== added file 'src/SBufDetailedStats.h'
--- src/SBufDetailedStats.h 1970-01-01 00:00:00 +0000
+++ src/SBufDetailedStats.h 2013-11-24 07:06:56 +0000
@@ -0,0 +1,27 @@
+
+#ifndef SQUID_SBUFDETAILEDSTATS_H
+#define SQUID_SBUFDETAILEDSTATS_H
+
+#include "SBuf.h"
+
+class StatHist;
+
+/// Record the size a SBuf had when it was destructed
+void recordSBufSizeAtDestruct(SBuf::size_type sz);
+
+/** Collect the SBuf size-at-destruct-time histogram
+ *
+ * \note the returned StatHist object must not be freed
+ */
+const StatHist * collectSBufDestructTimeStats();
+
+/// Record the size a MemBlob had when it was destructed
+void recordMemBlobSizeAtDestruct(MemBlob::size_type sz);
+
+/** Collect the MemBlob size-at-destruct-time histogram
+ *
+ * \note the returned StatHist object must not be freed
+ */
+const StatHist * collectMemBlobDestructTimeStats();
+
+#endif /* SQUID_SBUFDETAILEDSTATS_H */
=== modified file 'src/SBufStatsAction.cc'
--- src/SBufStatsAction.cc 2013-10-10 08:44:03 +0000
+++ src/SBufStatsAction.cc 2013-11-24 07:06:56 +0000
@@ -30,6 +30,7 @@
#include "ipc/Messages.h"
#include "ipc/TypedMsgHdr.h"
#include "mgr/Registration.h"
+#include "SBufDetailedStats.h"
#include "SBufStatsAction.h"
#include "StoreEntryStream.h"
@@ -48,6 +49,8 @@
{
sbdata += dynamic_cast<const SBufStatsAction&>(action).sbdata;
mbdata += dynamic_cast<const SBufStatsAction&>(action).mbdata;
+ sbsizesatdestruct += dynamic_cast<const SBufStatsAction&>(action).sbsizesatdestruct;
+ mbsizesatdestruct += dynamic_cast<const SBufStatsAction&>(action).mbsizesatdestruct;
}
void
@@ -55,17 +58,24 @@
{
sbdata = SBuf::GetStats();
mbdata = MemBlob::GetStats();
+ sbsizesatdestruct = *collectSBufDestructTimeStats();
+ mbsizesatdestruct = *collectMemBlobDestructTimeStats();
}
void
SBufStatsAction::dump(StoreEntry* entry)
{
StoreEntryStream ses(entry);
+ ses << "\n\n\nThese statistics are experimental; their format and contents "
+ "should not be relied upon, they are bound to change as "
+ "the SBuf feature is evolved\n";
sbdata.dump(ses);
mbdata.dump(ses);
- ses << "\n\n\nThese statistics are experimental; their format and contents "
- "should not be relied upon, they are bound to change as "
- "the SBuf feature is evolved";
+ ses << "\n";
+ ses << "SBuf size distribution at destruct time:\n";
+ sbsizesatdestruct.dump(entry,NULL);
+ ses << "MemBlob size distribution at destruct time:\n";
+ mbsizesatdestruct.dump(entry,NULL);
}
void
=== modified file 'src/SBufStatsAction.h'
--- src/SBufStatsAction.h 2013-10-10 08:44:03 +0000
+++ src/SBufStatsAction.h 2013-11-23 17:16:43 +0000
@@ -31,6 +31,7 @@
#include "mgr/Action.h"
#include "SBuf.h"
+#include "StatHist.h"
class StoreEntry;
@@ -55,6 +56,8 @@
SBufStats sbdata;
MemBlobStats mbdata;
+ StatHist sbsizesatdestruct;
+ StatHist mbsizesatdestruct;
};
#endif /* SQUID_SBUFSTATSACTION_H */
=== modified file 'src/StatHist.cc'
--- src/StatHist.cc 2013-01-21 07:15:09 +0000
+++ src/StatHist.cc 2013-11-23 11:47:06 +0000
@@ -205,6 +205,26 @@
}
}
+StatHist &
+StatHist::operator += (const StatHist &B)
+{
+ Must(capacity_ == B.capacity_);
+ Must(min_ == B.min_);
+ Must(max_ == B.max_);
+
+ if (B.bins == NULL) { // B was not yet initializted
+ return *this;
+ }
+ if (bins == NULL) { // this histogram was not yet initialized
+ *this = B;
+ return *this;
+ }
+ for (unsigned int i = 0; i < capacity_; ++i) {
+ bins[i] += B.bins[i];
+ }
+ return *this;
+}
+
/* log based histogram */
double
Math::Log(double x)
=== modified file 'src/StatHist.h'
--- src/StatHist.h 2012-08-28 13:00:30 +0000
+++ src/StatHist.h 2013-11-24 07:06:56 +0000
@@ -73,23 +73,36 @@
* this and the supplied histogram.
*/
double deltaPctile(const StatHist &B, double pctile) const;
+
/** obtain the output-transformed value from the specified bin
*
*/
double val(unsigned int bin) const;
+
/** increment the counter for the histogram entry
* associated to the supplied value
*/
void count(double val);
+
/** iterate the supplied bd function over the histogram values
*/
void dump(StoreEntry *sentry, StatHistBinDumper * bd) const;
+
/** Initialize the Histogram using a logarithmic values distribution
*/
void logInit(unsigned int capacity, double min, double max);
+
/** initialize the histogram to count occurrences in an enum-represented set
*/
void enumInit(unsigned int last_enum);
+
+ /** Import values from another histogram
+ *
+ * \note: the two histograms MUST have the same capicity, min and max or
+ * an exception will be raised
+ */
+ StatHist &operator += (const StatHist &B);
+
protected:
/** low-level initialize function. called by *Init high-level functions
* \note Important restrictions on val_in and val_out functions:
@@ -103,16 +116,21 @@
* See log and linear based histograms for examples
*/
void init(unsigned int capacity, hbase_f * val_in, hbase_f * val_out, double min, double max);
+
/// find what entry in the histogram corresponds to v, by applying
/// the preset input transformation function
unsigned int findBin(double v);
+
/// the histogram counters
bins_type *bins;
unsigned int capacity_;
+
/// minimum value to be stored, corresponding to the first bin
double min_;
+
/// value of the maximum counter in the histogram
double max_;
+
/// scaling factor when looking for a bin
double scale_;
hbase_f *val_in; /* e.g., log() for log-based histogram */
=== added file 'src/tests/stub_SBufDetailedStats.cc'
--- src/tests/stub_SBufDetailedStats.cc 1970-01-01 00:00:00 +0000
+++ src/tests/stub_SBufDetailedStats.cc 2013-11-22 22:24:51 +0000
@@ -0,0 +1,12 @@
+#include "squid.h"
+#include "SBuf.h"
+
+#define STUB_API "SBufDetailedStats.cc"
+#include "tests/STUB.h"
+
+class StatHist;
+
+void recordSBufSizeAtDestruct(SBuf::size_type) STUB_NOP
+const StatHist * collectSBufDestructTimeStats() STUB_RETVAL(NULL)
+void recordMemBlobSizeAtDestruct(SBuf::size_type) STUB_NOP
+const StatHist * collectMemBlobDestructTimeStats() STUB_RETVAL(NULL)
=== modified file 'src/tests/testSBuf.cc'
--- src/tests/testSBuf.cc 2013-10-25 00:13:46 +0000
+++ src/tests/testSBuf.cc 2013-11-24 08:16:48 +0000
@@ -1,8 +1,11 @@
#include "squid.h"
#include "Mem.h"
#include "SBuf.h"
+#include "SBufList.h"
#include "SBufFindTest.h"
#include "SBufStream.h"
+#include "SBufTokenizer.h"
+#include "SBufUtil.h"
#include "SquidString.h"
#include "testSBuf.h"
@@ -730,6 +755,26 @@
CPPUNIT_ASSERT_EQUAL(false,literal.startsWith(casebuf,caseInsensitive));
}
+void testSBuf::testSBufList()
+{
+ SBufList foo;
+ for (int j=0; j<sbuf_tokens_number; ++j)
+ foo.push_back(tokens[j]);
+ CPPUNIT_ASSERT(isMember(foo,SBuf("fox")));
+ CPPUNIT_ASSERT(isMember(foo,SBuf("Fox"),caseInsensitive));
+ CPPUNIT_ASSERT(!isMember(foo,SBuf("garble")));
+
+ SBuf joined=SBufListJoin(foo,SBuf(" "));
+ CPPUNIT_ASSERT_EQUAL(literal,joined);
+}
=== modified file 'src/tests/testSBuf.h'
--- src/tests/testSBuf.h 2013-07-26 09:20:09 +0000
+++ src/tests/testSBuf.h 2013-10-04 15:33:54 +0000
@@ -38,8 +38,11 @@
CPPUNIT_TEST( testPrintf );
CPPUNIT_TEST( testScanf );
CPPUNIT_TEST( testCopy );
CPPUNIT_TEST( testStringOps );
CPPUNIT_TEST( testGrow );
+ CPPUNIT_TEST( testSBufList );
CPPUNIT_TEST( testSBufStream );
CPPUNIT_TEST( testAutoFind );
CPPUNIT_TEST( testStdStringOps );
@@ -74,9 +77,12 @@
void testRFindSBuf();
void testSearchFail();
void testCopy();
void testStringOps();
void testGrow();
void testStartsWith();
+ void testSBufList();
void testSBufStream();
void testFindFirstOf();
void testAutoFind();
=== modified file 'src/tests/testStatHist.cc'
--- src/tests/testStatHist.cc 2013-10-25 00:13:46 +0000
+++ src/tests/testStatHist.cc 2013-11-23 11:47:06 +0000
@@ -73,3 +73,25 @@
test.count(max);
//CPPUNIT_ASSERT(test.val(capacity-1)==1); //FIXME: val() returns a density
}
+
+void
+testStatHist::testStatHistSum()
+{
+ InspectingStatHist s1, s2;
+ s1.logInit(30,1.0,100.0);
+ s2.logInit(30,1.0,100.0);
+ s1.count(3);
+ s2.count(30);
+ InspectingStatHist ts1, ts2;
+ ts1=s1;
+ ts1+=s2;
+ ts2=s2;
+ ts2+=s1;
+ CPPUNIT_ASSERT(ts1 == ts2);
+ InspectingStatHist ts3;
+ ts3.logInit(30,1.0,100.0);
+ ts3.count(3);
+ ts3.count(30);
+ CPPUNIT_ASSERT(ts3 == ts1);
+
+}
=== modified file 'src/tests/testStatHist.h'
--- src/tests/testStatHist.h 2012-08-28 13:00:30 +0000
+++ src/tests/testStatHist.h 2013-11-23 11:47:06 +0000
@@ -13,6 +13,7 @@
CPPUNIT_TEST( testStatHistBaseEquality );
CPPUNIT_TEST( testStatHistBaseAssignment );
CPPUNIT_TEST( testStatHistLog );
+ CPPUNIT_TEST( testStatHistSum );
CPPUNIT_TEST_SUITE_END();
public:
@@ -21,6 +22,7 @@
void testStatHistBaseEquality();
void testStatHistBaseAssignment();
void testStatHistLog();
+ void testStatHistSum();
};
#endif /* TESTSTATHIST_H_ */
