threads / patch / 8357

patch, 2 partsgitview: Define __slots__ for Commit

Subject: [PATCH 2/2] gitview: Define __slots__ for Commit

## tl;dr

4 messages between May 30, 2007 and Jun 4, 2007. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Michael Ellerman· May 30, 2007, 04:47 UTC · lore

[PATCH 1/2] gitview: Use new-style classes

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 contrib/gitview/gitview |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
Show changes to contrib/gitview/gitview +4 −4
diff --git a/contrib/gitview/gitview b/contrib/gitview/gitview
index 2d80e2b..6b54bc0 100755
--- a/contrib/gitview/gitview
+++ b/contrib/gitview/gitview
@@ -259,7 +259,7 @@ class CellRendererGraph(gtk.GenericCellRenderer):
 				self.set_colour(ctx, colour, 0.0, 0.5)
 			ctx.show_text(name)
 
-class Commit:
+class Commit(object):
 	""" This represent a commit object obtained after parsing the git-rev-list
 	output """
 
@@ -339,7 +339,7 @@ class Commit:
 		fp.close()
 		return diff
 
-class AnnotateWindow:
+class AnnotateWindow(object):
 	"""Annotate window.
 	This object represents and manages a single window containing the
 	annotate information of the file
@@ -519,7 +519,7 @@ class AnnotateWindow:
 		self.io_watch_tag = gobject.io_add_watch(fp, gobject.IO_IN, self.data_ready)
 
 
-class DiffWindow:
+class DiffWindow(object):
 	"""Diff window.
 	This object represents and manages a single window containing the
 	differences between two revisions on a branch.
@@ -674,7 +674,7 @@ class DiffWindow:
 			fp.close()
 		dialog.destroy()
 
-class GitView:
+class GitView(object):
 	""" This is the main class
 	"""
 	version = "0.9"

-- 
1.5.1.3.g7a33b
Michael Ellerman· May 30, 2007, 04:47 UTC · re: Michael Ellerman · lore

Define __slots__ for the Commit class. This reserves space in each Commit object for only the defined variables. On my system this reduces heap usage when viewing a kernel repo by 12% ~= 55868 KB.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 contrib/gitview/gitview |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
Show changes to contrib/gitview/gitview +3 −0
diff --git a/contrib/gitview/gitview b/contrib/gitview/gitview
index 6b54bc0..4baa1dd 100755
--- a/contrib/gitview/gitview
+++ b/contrib/gitview/gitview
@@ -263,6 +263,9 @@ class Commit(object):
 	""" This represent a commit object obtained after parsing the git-rev-list
 	output """
 
+	__slots__ = ['children_sha1', 'message', 'author', 'date', 'committer',
+				 'commit_date', 'commit_sha1', 'parent_sha1']
+
 	children_sha1 = {}
 
 	def __init__(self, commit_lines):

-- 
1.5.1.3.g7a33b
Junio C Hamano· Jun 2, 2007, 09:38 UTC · re: Michael Ellerman · lore

Re: [PATCH 1/2] gitview: Use new-style classes

Michael Ellerman <michael@ellerman.id.au> writes:
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Aneesh, this one is in your bailiwick.

Michael, it would have been nicer to have a real log message, not just the Subject line, in your message. The Subject line says what the patch does in what subarea of the git.git tree, which is very appropriate, but that can be seen from the diff even if you did not say so. The log message (before your Signed-off-by line) is where you describe _why_ you would think it is a good change. "What" and "how" are usually evident in well written code without much explanation, but "why" is often more useful when reviewing the patch, and reading and studying the history of the code.

I would have written something like:
	This changes the Commit class to use new-style class,
	which has been available since Python 2.2 (Dec 2001).
	This is a necessary step in order to use __slots__
	declaration so that we can reduce the memory footprint
	with my next patch.

I have no strong preference on this change myself. "Classes that derive from type" are relatively recent invention but I would personally feel that Python 2.2 is ancient enough that using it would not pose any practical portability issues, but what would I know...

The __slots__ patch depends on this, and that one has a measurable memory footprint improvement, so I would say we should take it.

Michael Ellerman· Jun 4, 2007, 05:54 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/2] gitview: Use new-style classes

On Sat, 2007-06-02 at 02:38 -0700, Junio C Hamano wrote:
Show 30 quoted lines
> Michael Ellerman <michael@ellerman.id.au> writes:
> 
> > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> 
> Aneesh, this one is in your bailiwick.
> 
> Michael, it would have been nicer to have a real log message,
> not just the Subject line, in your message.  The Subject line
> says what the patch does in what subarea of the git.git tree,
> which is very appropriate, but that can be seen from the diff
> even if you did not say so.  The log message (before your
> Signed-off-by line) is where you describe _why_ you would think
> it is a good change.  "What" and "how" are usually evident in
> well written code without much explanation, but "why" is often
> more useful when reviewing the patch, and reading and studying
> the history of the code.
> 
> I would have written something like:
> 
> 	This changes the Commit class to use new-style class,
> 	which has been available since Python 2.2 (Dec 2001).
> 	This is a necessary step in order to use __slots__
> 	declaration so that we can reduce the memory footprint
> 	with my next patch.
> 
> I have no strong preference on this change myself.  "Classes
> that derive from type" are relatively recent invention but I
> would personally feel that Python 2.2 is ancient enough that
> using it would not pose any practical portability issues, but
> what would I know...
Hi Junio,
Why write my own changelog when you do such a good job! ;)

But seriously, sorry about that. It was a case of me hacking on my local git tree, not thinking anything would come of it, and then when I saw the memory reduction from using __slots__ I decided to send off what I'd done. I should have reset and recommitted with a decent changelog.

cheers
-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

← back to recent threads