Reduce nr of ptr derefs in fs/jffs2/summary.c

PATCH In review Jesper Juhl 3 participants 4 messages Dec 11, 2005 – Dec 16, 2005 Journalling flash file system V2 (JFFS2)

lore

Share & tools

lore

Jesper Juhl

author

2005-12-11 05:40 UTC #0 lore raw

PATCH 1/1 Reduce nr of ptr derefs in fs/jffs2/summary.c

Here's a small patch to reduce the nr. of pointer dereferences in fs/jffs2/summary.c

Benefits:
- micro speed optimization due to fewer pointer derefs
- generated code is slightly smaller
- better readability

Please consider applying...

Signed-off-by Jesper Juhl <[email protected]>

---

 fs/jffs2/summary.c |   38 +++++++++++++++++++-------------------
 1 files changed, 19 insertions(+), 19 deletions(-)

orig:
text data bss dec hex filename
5033 0 0 5033 13a9 fs/jffs2/summary.o
patched:
text data bss dec hex filename
4935 0 0 4935 1347 fs/jffs2/summary.o

fs/jffs2/summary.c +19 ~~-19~~

@@ -581,16 +581,17 @@ static int jffs2_sum_write_data(struct j
 	wpage = c->summary->sum_buf;
 
 	while (c->summary->sum_num) {
+		temp = c->summary->sum_list_head;
 
-		switch (je16_to_cpu(c->summary->sum_list_head->u.nodetype)) {
+		switch (je16_to_cpu(temp->u.nodetype)) {
 			case JFFS2_NODETYPE_INODE: {
 				struct jffs2_sum_inode_flash *sino_ptr = wpage;
 
-				sino_ptr->nodetype = c->summary->sum_list_head->i.nodetype;
-				sino_ptr->inode = c->summary->sum_list_head->i.inode;
-				sino_ptr->version = c->summary->sum_list_head->i.version;
-				sino_ptr->offset = c->summary->sum_list_head->i.offset;
-				sino_ptr->totlen = c->summary->sum_list_head->i.totlen;
+				sino_ptr->nodetype = temp->i.nodetype;
+				sino_ptr->inode = temp->i.inode;
+				sino_ptr->version = temp->i.version;
+				sino_ptr->offset = temp->i.offset;
+				sino_ptr->totlen = temp->i.totlen;
 
 				wpage += JFFS2_SUMMARY_INODE_SIZE;

@@ -600,19 +601,19 @@ static int jffs2_sum_write_data(struct j
 			case JFFS2_NODETYPE_DIRENT: {
 				struct jffs2_sum_dirent_flash *sdrnt_ptr = wpage;
 
-				sdrnt_ptr->nodetype = c->summary->sum_list_head->d.nodetype;
-				sdrnt_ptr->totlen = c->summary->sum_list_head->d.totlen;
-				sdrnt_ptr->offset = c->summary->sum_list_head->d.offset;
-				sdrnt_ptr->pino = c->summary->sum_list_head->d.pino;
-				sdrnt_ptr->version = c->summary->sum_list_head->d.version;
-				sdrnt_ptr->ino = c->summary->sum_list_head->d.ino;
-				sdrnt_ptr->nsize = c->summary->sum_list_head->d.nsize;
-				sdrnt_ptr->type = c->summary->sum_list_head->d.type;
+				sdrnt_ptr->nodetype = temp->d.nodetype;
+				sdrnt_ptr->totlen = temp->d.totlen;
+				sdrnt_ptr->offset = temp->d.offset;
+				sdrnt_ptr->pino = temp->d.pino;
+				sdrnt_ptr->version = temp->d.version;
+				sdrnt_ptr->ino = temp->d.ino;
+				sdrnt_ptr->nsize = temp->d.nsize;
+				sdrnt_ptr->type = temp->d.type;
 
-				memcpy(sdrnt_ptr->name, c->summary->sum_list_head->d.name,
-							c->summary->sum_list_head->d.nsize);
+				memcpy(sdrnt_ptr->name, temp->d.name,
+							temp->d.nsize);
 
-				wpage += JFFS2_SUMMARY_DIRENT_SIZE(c->summary->sum_list_head->d.nsize);
+				wpage += JFFS2_SUMMARY_DIRENT_SIZE(temp->d.nsize);
 
 				break;
 			}

@@ -622,8 +623,7 @@ static int jffs2_sum_write_data(struct j
 			}
 		}
 
-		temp = c->summary->sum_list_head;
-		c->summary->sum_list_head = c->summary->sum_list_head->u.next;
+		c->summary->sum_list_head = temp->u.next;
 		kfree(temp);
 
 		c->summary->sum_num--;

Jan Engelhardt

2005-12-15 22:55 UTC #1 lore raw

Quoted 4 lines

Benefits:
- micro speed optimization due to fewer pointer derefs
- generated code is slightly smaller

Should not these two at best be done by the compiler?

Jan Engelhardt

Andy Isaacson

2005-12-16 03:34 UTC #2 lore raw

Quoted 5 lines · Jan Engelhardt

Quoted 3 lines
Benefits:
- micro speed optimization due to fewer pointer derefs
- generated code is slightly smaller
Should not these two at best be done by the compiler?

The compiler cannot, in general, do CSE on pointer dereferences. Consider the following snippet from fs/jffs2/summary.c, both before

610 sdrnt_ptr->type = c->summary->sum_list_head->d.type;
612 memcpy(sdrnt_ptr->name, c->summary->sum_list_head->d.name,
613 c->summary->sum_list_head->d.nsize);
615 wpage += JFFS2_SUMMARY_DIRENT_SIZE(c->summary->sum_list_head->d.nsize);

and after Jesper's patch:

611 sdrnt_ptr->type = temp->d.type;
613 memcpy(sdrnt_ptr->name, temp->d.name,
614 temp->d.nsize);
616 wpage += JFFS2_SUMMARY_DIRENT_SIZE(temp->d.nsize);

Assuming that memcpy is an out-of-line function, the compiler has to handle the worst case, that it might modify c->summary->sum_list_head and thus make the saved value stale. (In the specific case of memcpy the compiler can take advantage of special knowledge about the function, and it's probably inline anyways, so this *particular* example is not real; but it's a nice clean classroom example.)

But a human *can* make the obvious leap and tell the compiler that the value can be computed once and then saved. And besides, isn't the code just *much* nicer to look at, above?

-andy

Jan Engelhardt

2005-12-16 22:05 UTC #3 lore raw

Quoted 10 lines

Quoted 5 lines
Quoted 3 lines
Benefits:
- micro speed optimization due to fewer pointer derefs
- generated code is slightly smaller
Should not these two at best be done by the compiler?
[...]
But a human *can* make the obvious leap and tell the compiler that the value can be computed once and then saved. And besides, isn't the code just *much* nicer to look at, above?

The third point Jesper mentioned was about readability, but I never questioned that.

Jan Engelhardt