mosquitto icon indicating copy to clipboard operation
mosquitto copied to clipboard

Feature: Add support for publishing multiple buffers concatenated together

Open sidhant007 opened this issue 4 years ago • 2 comments

As discussed in my previous PR (which I broke accidentally while force-pushing an unintended commit and subsequently closed that PR)

Motivation: If you want to make a single publish to consist of multiple buffers concatenated together, then instead of concatenating the buffers on the user's end and then passing it to mosquitto_publish the user can call mosquitto_publish_bufs, which will reduce memcpy calls on the user's end. Example - If the user has 10 buffers each 20 bytes big, then the user will save cost (in terms of both, time and memory) of doing memcpy for a total of 200 bytes.

Signature of the function: int mosquitto_publish_bufs(mosq, mid, topic, buffers, buffers_cnt, qos, retain), where buffers is an array of struct buf, where

struct buf {
   const void *base;
   uint32_t len;
};

^Taking into account the feedback provided on the last PR.

The difference in implementations for users will look like following:

With publish_bufs:

buffer_count = sensor_count + 2;
buffers = malloc(buffer_count * sizeof(struct buf));
buffer[0]->iov_base = header;
buffer[0]->iov_len = strlen(header);
for(i=0; i<sensor_count; i++){
    buffer[i+1]->base = sensor[i];
    buffer[i+1]->len = strlen(sensor[i]);
}
for(i=0;i<4;i++){
	buffers[i].base = message[i];
	buffers[i].len = strlen(message[i]);
}
mosquitto_publish_bufs(..., buffers, buffer_count, ...);
free(buffers);

Without publish_bufs:

payload_len = strlen(header)+strlen(footer);
for(i=0; i<sensor_count, i++){
	payload_len += strlen(sensor[i]);
}
payload = malloc(payloadlen + 1);
memcpy(payload, header, strlen(header));
offset += strlen(header);
for(i=0; i<sensor_count; i++){
	len = strlen(sensor[i]);
	memcpy(&payload[offset], sensor[i], len);
	offset += len;
}
memcpy(&payload[offset], footer, strlen(footer));
mosquitto_publish(..., payload_len, payload, ...);
free(payload);
  • I have defined a new struct buf instead of using struct iovec (present in <sys/uio.h>) because of the feedback provided here of keeping the mosquitto API as consistent as possible across platforms.
  • The PR is big in size because of the tests added, if more convenient to review, then let me know and I can break this into multiple PRs (4 = 1 for feature + 3 for new tests added).
  • The implementation makes mosquitto_publish a special case of mosquitto_publish_bufs to reduce code-duplication. I can make the two separate in-case the maintainer prefers so.
  • The three new tests added (03-publish_bufs-qos0, 03-publish_bufs-qos0-no-payload, 03-publish_bufs-c2b-qos1-disconnect) extend the code-coverage to the new functionality added (primarily when buffers_cnt != 1) and the currently present (03-publish-*) tests still pass confirming that mosquitto_publish_bufs works fine when buffers_cnt == 1.

  • [X] Have you signed the Eclipse Contributor Agreement, using the same email address as you used in your commits?
  • [X] Do each of your commits have a "Signed-off-by" line, with the correct email address? Use "git commit -s" to generate this line for you.
  • [X] If you are contributing a new feature, is your work based off the develop branch?
  • [X] Have you added an explanation of what your changes do and why you'd like us to include them?
  • [X] Have you successfully run make test with your changes locally?

sidhant007 avatar Jul 02 '20 05:07 sidhant007

@ralight, please let me know if you have some feedback for this PR.

sidhant007 avatar Jul 16 '20 02:07 sidhant007

I am interested in this PR too. Can we have a review?

zadewg avatar Jul 31 '23 06:07 zadewg