Citeren Alexandru Ardelean <ardeleana...@gmail.com>:

On Tue, Nov 7, 2017 at 11:18 PM, Arjen de Korte <arjen+l...@de-korte.org> wrote:
Citeren Rosen Penev <ros...@gmail.com>:

I beg to differ. https://vorpus.org/blog/why-does-calloc-exist/

Section 2.


I don't care about theoretical gains, benchmarks please. How much do you
gain with these patches? I really doubt that in any of these patches there
will be a tangible gain.

This feels like an opinion battle :)

I'd also vote for calloc [vs malloc + memset] mostly for reduction of
line numbers and/or cleaning up.

I also feel that Rosen's argument for "faster in extreme situations"
is somewhat "playing it by the ear".
But I would not dismiss this based on that.

These days compilers do so much optimization, it's hard to evaluate
performance of one case vs another.
For this case [specifically], we would need to see/check the assembly
code for a few architectures [mips, powerpc, arm, x86, etc] to
properly evaluate the performance improvement of calloc() vs malloc()
+ memset().
I feel that effort would be a too much for such a simple/trivial change.

I would re-spin this with just the "cleaning up" part [since I feel
there is more agreement on that].

But feel free do disagree with me :)

I really see no point in implementing these patches.

I've seen the backlash of far too many efficiency improvements, that either didn't live up to the expectations or outright introduced new bugs, that for anyone claiming something to be better, I want to see proof that it actually improves something *and* does not break something else. The commit messages I have seen so far provide neither.

"If it ain't broke, don't fix it".


On Tue, Nov 7, 2017 at 12:46 PM, Arjen de Korte <arjen+l...@de-korte.org>
wrote:

Citeren Rosen Penev <ros...@gmail.com>:

Replace malloc+memset with calloc. Cleaner and faster in extreme
situations.



Calloc is definitly *not* faster than malloc + memset. Under the hood,
calloc will call malloc, check if memory allocation was successful and
then
proceed to set all allocated memory to 0. You need to check the return
value
of malloc or calloc anyway, so this will be actually slower.

It *may* be considered cleaner, since it will check if the memory
allocation
succeeded before writing zeros.


Signed-off-by: Rosen Penev <ros...@gmail.com>
---
 libubus.c  |  6 ++----
 lua/ubus.c | 18 ++++++------------
 2 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/libubus.c b/libubus.c
index 9463522..260e40f 100644
--- a/libubus.c
+++ b/libubus.c
@@ -133,7 +133,7 @@ struct ubus_lookup_request {
 static void ubus_lookup_cb(struct ubus_request *ureq, int type, struct
blob_attr *msg)
 {
        struct ubus_lookup_request *req;
-       struct ubus_object_data obj;
+       struct ubus_object_data obj = {};
        struct blob_attr **attr;

        req = container_of(ureq, struct ubus_lookup_request, req);
@@ -143,7 +143,6 @@ static void ubus_lookup_cb(struct ubus_request
*ureq,
int type, struct blob_attr
            !attr[UBUS_ATTR_OBJTYPE])
                return;

-       memset(&obj, 0, sizeof(obj));
        obj.id = blob_get_u32(attr[UBUS_ATTR_OBJID]);
        obj.path = blob_data(attr[UBUS_ATTR_OBJPATH]);
        obj.type_id = blob_get_u32(attr[UBUS_ATTR_OBJTYPE]);
@@ -220,7 +219,7 @@ int ubus_register_event_handler(struct ubus_context
*ctx,
                                const char *pattern)
 {
        struct ubus_object *obj = &ev->obj;
-       struct blob_buf b2;
+       struct blob_buf b2 = {};
        int ret;

        if (!obj->id) {
@@ -236,7 +235,6 @@ int ubus_register_event_handler(struct ubus_context
*ctx,
        }

        /* use a second buffer, ubus_invoke() overwrites the primary one
*/
-       memset(&b2, 0, sizeof(b2));
        blob_buf_init(&b2, 0);
        blobmsg_add_u32(&b2, "object", obj->id);
        if (pattern)
diff --git a/lua/ubus.c b/lua/ubus.c
index 74a15b0..e9fd10e 100644
--- a/lua/ubus.c
+++ b/lua/ubus.c
@@ -410,11 +410,10 @@ static int ubus_lua_load_methods(lua_State *L,
struct ubus_method *m)
        }

        /* setup the policy pointers */
-       p = malloc(sizeof(struct blobmsg_policy) * plen);
+       p = calloc(plen, sizeof(struct blobmsg_policy));
        if (!p)
                return 1;

-       memset(p, 0, sizeof(struct blobmsg_policy) * plen);
        m->policy = p;
        lua_pushnil(L);
        while (lua_next(L, -2) != 0) {
@@ -481,26 +480,23 @@ static struct ubus_object*
ubus_lua_load_object(lua_State *L)
        int midx = 0;

        /* setup object pointers */
-       obj = malloc(sizeof(struct ubus_lua_object));
+       obj = calloc(1, sizeof(struct ubus_lua_object));
        if (!obj)
                return NULL;

-       memset(obj, 0, sizeof(struct ubus_lua_object));
        obj->o.name = lua_tostring(L, -2);

        /* setup method pointers */
-       m = malloc(sizeof(struct ubus_method) * mlen);
-       memset(m, 0, sizeof(struct ubus_method) * mlen);
+       m = calloc(1, sizeof(struct ubus_method) * mlen);
        obj->o.methods = m;

        /* setup type pointers */
-       obj->o.type = malloc(sizeof(struct ubus_object_type));
+       obj->o.type = calloc(1, sizeof(struct ubus_object_type));
        if (!obj->o.type) {
                free(obj);
                return NULL;
        }

-       memset(obj->o.type, 0, sizeof(struct ubus_object_type));
        obj->o.type->name = lua_tostring(L, -2);
        obj->o.type->id = 0;
        obj->o.type->methods = obj->o.methods;
@@ -715,11 +711,10 @@ ubus_lua_load_event(lua_State *L)
 {
        struct ubus_lua_event* event = NULL;

-       event = malloc(sizeof(struct ubus_lua_event));
+       event = calloc(1, sizeof(struct ubus_lua_event));
        if (!event)
                return NULL;

-       memset(event, 0, sizeof(struct ubus_lua_event));
        event->e.cb = ubus_event_handler;

        /* update the he callback lookup table */
@@ -818,8 +813,7 @@ ubus_lua_do_subscribe( struct ubus_context *ctx,
lua_State *L, const char* targe
                lua_error( L );
        }

-       sub = malloc( sizeof( struct ubus_lua_subscriber ) );
-       memset( sub, 0, sizeof( struct ubus_lua_subscriber ) );
+       sub = calloc( 1, sizeof( struct ubus_lua_subscriber ) );
        if( !sub ){
                lua_pushstring( L, "Out of memory" );
                lua_error( L );






_______________________________________________
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev





_______________________________________________
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev

_______________________________________________
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev




_______________________________________________
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev

Reply via email to