Skip to content

Commit

Permalink
Replace MEM_DWRITE_Q pool with MEMPROXY class pool (#1873)
Browse files Browse the repository at this point in the history
Replacing memory allocate and destruct with C++ new/delete
and in-class initialization.
  • Loading branch information
yadij authored and squid-anubis committed Oct 20, 2024
1 parent 6c33c64 commit ed1a732
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 60 deletions.
84 changes: 33 additions & 51 deletions src/fs_io.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,26 @@ static void cxx_xfree(void *ptr)
xfree(ptr);
}

dwrite_q::dwrite_q(const size_t aSize, char * const aBuffer, FREE * const aFree):
buf(aBuffer),
capacity(aSize),
free_func(aFree)
{
assert(buf || !free_func);
if (!buf) {
buf = static_cast<char *>(xmalloc(aSize));
free_func = cxx_xfree; // dwrite_q buffer xfree()
} else {
len = aSize;
}
}

dwrite_q::~dwrite_q()
{
if (free_func)
free_func(buf);
}

/*
* opens a disk file specified by 'path'. This function always
* blocks! There is no callback.
Expand Down Expand Up @@ -133,35 +153,18 @@ diskCombineWrites(_fde_disk *fdd)
*/

if (fdd->write_q != nullptr && fdd->write_q->next != nullptr) {
int len = 0;
size_t wantCapacity = 0;

for (dwrite_q *q = fdd->write_q; q != nullptr; q = q->next)
len += q->len - q->buf_offset;

dwrite_q *wq = (dwrite_q *)memAllocate(MEM_DWRITE_Q);

wq->buf = (char *)xmalloc(len);

wq->len = 0;

wq->buf_offset = 0;
wantCapacity += q->len - q->buf_offset; // XXX: might overflow

wq->next = nullptr;

wq->free_func = cxx_xfree;

while (fdd->write_q != nullptr) {
dwrite_q *q = fdd->write_q;

len = q->len - q->buf_offset;
const auto wq = new dwrite_q(wantCapacity);
while (const auto q = fdd->write_q) {
const auto len = q->len - q->buf_offset;
memcpy(wq->buf + wq->len, q->buf + q->buf_offset, len);
wq->len += len;
fdd->write_q = q->next;

if (q->free_func)
q->free_func(q->buf);

memFree(q, MEM_DWRITE_Q);
delete q;
};

fdd->write_q_tail = wq;
Expand All @@ -178,11 +181,10 @@ diskHandleWrite(int fd, void *)
fde *F = &fd_table[fd];

_fde_disk *fdd = &F->disk;
dwrite_q *q = fdd->write_q;
int status = DISK_OK;
bool do_close;

if (nullptr == q)
if (!fdd->write_q)
return;

debugs(6, 3, "diskHandleWrite: FD " << fd);
Expand Down Expand Up @@ -246,23 +248,16 @@ diskHandleWrite(int fd, void *)
* repeated write failures for the same FD because of
* the queued data.
*/
do {
while (const auto q = fdd->write_q) {
fdd->write_q = q->next;

if (q->free_func)
q->free_func(q->buf);

if (q) {
memFree(q, MEM_DWRITE_Q);
q = nullptr;
}
} while ((q = fdd->write_q));
delete q;
}
}

len = 0;
}

if (q != nullptr) {
if (const auto q = fdd->write_q) {
/* q might become NULL from write failure above */
q->buf_offset += len;

Expand All @@ -276,14 +271,7 @@ diskHandleWrite(int fd, void *)
if (q->buf_offset == q->len) {
/* complete write */
fdd->write_q = q->next;

if (q->free_func)
q->free_func(q->buf);

if (q) {
memFree(q, MEM_DWRITE_Q);
q = nullptr;
}
delete q;
}
}

Expand Down Expand Up @@ -331,18 +319,12 @@ file_write(int fd,
void *handle_data,
FREE * free_func)
{
dwrite_q *wq = nullptr;
fde *F = &fd_table[fd];
assert(fd >= 0);
assert(F->flags.open);
/* if we got here. Caller is eligible to write. */
wq = (dwrite_q *)memAllocate(MEM_DWRITE_Q);
const auto wq = new dwrite_q(len, static_cast<char *>(const_cast<void *>(ptr_to_buf)), free_func);
wq->file_offset = file_offset;
wq->buf = (char *)ptr_to_buf;
wq->len = len;
wq->buf_offset = 0;
wq->next = nullptr;
wq->free_func = free_func;

if (!F->disk.wrt_handle_data) {
F->disk.wrt_handle = handle;
Expand Down
23 changes: 16 additions & 7 deletions src/fs_io.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,25 @@ class dread_ctrl
void *client_data;
};

// POD
class dwrite_q
{
MEMPROXY_CLASS(dwrite_q);
public:
off_t file_offset;
char *buf;
size_t len;
size_t buf_offset;
dwrite_q *next;
FREE *free_func;
dwrite_q(const size_t wantCapacity) : dwrite_q(wantCapacity, nullptr, nullptr) {}
dwrite_q(size_t, char *, FREE *);
dwrite_q(dwrite_q &&) = delete; // no copying or moving of any kind
~dwrite_q();

off_t file_offset = 0;
char *buf = nullptr;
size_t len = 0; ///< length of content in buf
size_t buf_offset = 0;
dwrite_q *next = nullptr;

private:
size_t capacity = 0; ///< allocation size of buf
/// when set, gets called upon object destruction to free buf
FREE *free_func = nullptr;
};

int file_open(const char *path, int mode);
Expand Down
1 change: 0 additions & 1 deletion src/mem/forward.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ typedef enum {
MEM_32K_BUF,
MEM_64K_BUF,
MEM_DREAD_CTRL,
MEM_DWRITE_Q,
MEM_MD5_DIGEST,
MEM_MAX
} mem_type;
Expand Down
1 change: 0 additions & 1 deletion src/mem/old_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,6 @@ Mem::Init(void)
memDataInit(MEM_64K_BUF, "64K Buffer", 65536, 10, false);
// TODO: Carefully stop zeroing these objects memory and drop the doZero parameter
memDataInit(MEM_DREAD_CTRL, "dread_ctrl", sizeof(dread_ctrl), 0, true);
memDataInit(MEM_DWRITE_Q, "dwrite_q", sizeof(dwrite_q), 0, true);
memDataInit(MEM_MD5_DIGEST, "MD5 digest", SQUID_MD5_DIGEST_LENGTH, 0, true);
GetPool(MEM_MD5_DIGEST)->setChunkSize(512 * 1024);

Expand Down

0 comments on commit ed1a732

Please sign in to comment.