Skip to content

Commit

Permalink
Avoid malloc when creating strings in ext_fb
Browse files Browse the repository at this point in the history
Using StringData's ability to reserve space, we can steer more
string allocation traffic to small strings and smart_malloc'd strings,
at clean up code at the same time.

This also fixes a missing request-memory-limit check in smart_realloc.
  • Loading branch information
edwinsmith authored and sgolemon committed Oct 5, 2012
1 parent 448d0e9 commit a40f2a8
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 28 deletions.
3 changes: 3 additions & 0 deletions src/runtime/base/memory/memory_manager.cpp
Expand Up @@ -350,6 +350,9 @@ inline void* MemoryManager::smartRealloc(void* ptr, size_t nbytes) {
SweepNode* n2 = (SweepNode*) realloc(n, nbytes + sizeof(SweepNode));
if (n2 != n) {
// block moved; must re-link to sweeplist
if (hhvm && UNLIKELY(m_stats.usage > m_stats.maxBytes)) {
refreshStatsHelper();
}
if (next != n) {
next->prev = prev->next = n2;
} else {
Expand Down
18 changes: 18 additions & 0 deletions src/runtime/base/string_data.cpp
Expand Up @@ -388,6 +388,24 @@ MutableSlice StringData::reserve(int cap) {
return MutableSlice(m_data, cap);
}

StringData* StringData::shrink(int len) {
setSize(len);
switch (format()) {
case IsSmart:
m_data = (char*) smart_realloc(m_data, len + 1);
m_big.cap = len | IsSmart;
break;
case IsMalloc:
m_data = (char*) realloc(m_data, len + 1);
m_big.cap = len | IsMalloc;
break;
default:
// don't shrink
break;
}
return this;
}

StringData *StringData::copy(bool sharedMemory /* = false */) const {
if (isStatic()) {
// Static strings cannot change, and are always available.
Expand Down
4 changes: 3 additions & 1 deletion src/runtime/base/string_data.h
Expand Up @@ -220,10 +220,12 @@ class StringData {
return isSmall() ? MutableSlice(m_small, MaxSmallSize) :
MutableSlice(m_data, bigCap());
}
void setSize(int len) {
StringData* shrink(int len); // setSize and maybe realloc
StringData* setSize(int len) {
ASSERT(len >= 0 && len <= capacity() && !isImmutable());
m_data[len] = 0;
m_len = len;
return this;
}

~StringData() { releaseData(); }
Expand Down
5 changes: 5 additions & 0 deletions src/runtime/base/type_string.h
Expand Up @@ -207,6 +207,11 @@ class String : protected StringBase {
m_px->setSize(len);
return *this;
}
CStrRef shrink(int len) {
ASSERT(m_px);
m_px->shrink(len);
return *this;
}
const char *c_str() const {
return m_px ? m_px->data() : "";
}
Expand Down
42 changes: 15 additions & 27 deletions src/runtime/ext/ext_fb.cpp
Expand Up @@ -419,13 +419,11 @@ Variant f_fb_thrift_serialize(CVarRef thing) {
if (fb_serialized_size(thing, 0, &len)) {
return null;
}
char *buff = (char *)malloc(len + 1);
String s(len, ReserveString);
int pos = 0;
fb_serialize_into_buffer(thing, buff, &pos);

fb_serialize_into_buffer(thing, s.mutableSlice().ptr, &pos);
ASSERT(pos == len);
buff[len] = '\0';
return String(buff, len, AttachString);
return s.setSize(len);
}

int fb_compact_unserialize_from_buffer(
Expand Down Expand Up @@ -767,9 +765,9 @@ Variant f_fb_compact_serialize(CVarRef thing) {
if (thing.getType() == KindOfInt64) {
int64_t val = thing.toInt64();
if (val >= 0 && (uint64_t)val <= kInt7Mask) {
char* buf = (char*)malloc(2);
*(uint16_t*)(buf) = (uint16_t)htons(kInt13Prefix | val);
return String(buf, 2, AttachString);
String s(2, ReserveString);
*(uint16_t*)(s.mutableSlice().ptr) = (uint16_t)htons(kInt13Prefix | val);
return s.setSize(2);
}
}

Expand All @@ -781,7 +779,7 @@ Variant f_fb_compact_serialize(CVarRef thing) {
return null;
}

return String(sd);
return Variant(sd);
}

int fb_compact_unserialize_int64_from_buffer(
Expand Down Expand Up @@ -1176,7 +1174,7 @@ bool f_fb_utf8ize(VRefParam input) {
return false; // Too long.
}

// Preflight to avoid malloc() if the entire input is valid.
// Preflight to avoid allocation if the entire input is valid.
int32_t srcPosBytes;
for (srcPosBytes = 0; srcPosBytes < srcLenBytes; /* U8_NEXT increments */) {
// This is lame, but gcc doesn't optimize U8_NEXT very well
Expand Down Expand Up @@ -1212,10 +1210,8 @@ bool f_fb_utf8ize(VRefParam input) {
if (dstMaxLenBytes > INT_MAX) {
return false; // Too long.
}
char *dstBuf = (char*)malloc(dstMaxLenBytes + 1);
if (!dstBuf) {
return false;
}
String dstStr(dstMaxLenBytes, ReserveString);
char *dstBuf = dstStr.mutableSlice().ptr;

// Copy valid bytes found so far as one solid block.
memcpy(dstBuf, srcBuf, srcPosBytes);
Expand All @@ -1241,8 +1237,7 @@ bool f_fb_utf8ize(VRefParam input) {
// We know that resultBuffer > total possible length.
U8_APPEND_UNSAFE(dstBuf, dstPosBytes, curCodePoint);
}
dstBuf[dstPosBytes] = '\0';
input = String(dstBuf, dstPosBytes, AttachString);
input = dstStr.setSize(dstPosBytes);
return true;
}

Expand Down Expand Up @@ -1287,9 +1282,7 @@ int64 f_fb_utf8_strlen_deprecated(CStrRef input) {
static Variant f_fb_utf8_substr_simple(CStrRef str, int32_t firstCodePoint,
int32_t numDesiredCodePoints) {
const char* const srcBuf = str.data();
char *dstBuf;
int32_t srcLenBytes = str.size(); // May truncate; checked before use below.
int32_t dstPosBytes = 0;

ASSERT(firstCodePoint >= 0); // Wrapper fixes up negative starting positions.
ASSERT(numDesiredCodePoints > 0); // Wrapper fixes up negative/zero length.
Expand Down Expand Up @@ -1319,10 +1312,9 @@ static Variant f_fb_utf8_substr_simple(CStrRef str, int32_t firstCodePoint,
if (dstMaxLenBytes > INT_MAX) {
return false; // Too long.
}
dstBuf = (char*)malloc(dstMaxLenBytes + 1);
if (!dstBuf) {
return false;
}
String dstStr(dstMaxLenBytes, ReserveString);
char* dstBuf = dstStr.mutableSlice().ptr;
int32_t dstPosBytes = 0;

// Iterate through src's codepoints; srcPosBytes is incremented by U8_NEXT.
for (int32_t srcPosBytes = 0, srcPosCodePoints = 0;
Expand All @@ -1346,12 +1338,8 @@ static Variant f_fb_utf8_substr_simple(CStrRef str, int32_t firstCodePoint,
}

if (dstPosBytes > 0) {
ASSERT(dstPosBytes <= (int32_t)dstMaxLenBytes);
dstBuf[dstPosBytes] = '\0';
return String(dstBuf, dstPosBytes, AttachString);
return dstStr.setSize(dstPosBytes);
}

free(dstBuf);
return false;
}

Expand Down

0 comments on commit a40f2a8

Please sign in to comment.