[PATCH] memory-report: support calculating size for structures

[PATCH] memory-report: support calculating size for structures

[Yikai Zhao]

[-- Attachment #1: Type: text/plain, Size: 452 bytes --]

Hi,

Here's another patch on memory-report package which fixes the object size calculation for structures (cl-defstruct types). Before this patch, memory-report--object-size will report empty size for structure objects, which results in incorrect report about "Largest Variables" in memory-report result.

Regarding the copyright assignment, I'm currently in the progress of signing it. But maybe this patch is small enough to apply without it?


Yikai

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-memory-report-support-calculating-size-for-structure.patch --]
[-- Type: text/x-patch; name="0001-memory-report-support-calculating-size-for-structure.patch", Size: 2236 bytes --]

From 6a851e089d0bd9ef61d9209c5d3eb3932c7a7a6c Mon Sep 17 00:00:00 2001
From: Yikai Zhao <yikai@z1k.dev>
Date: Sun, 29 Aug 2021 21:47:12 +0800
Subject: [PATCH] memory-report: support calculating size for structures

---
 lisp/emacs-lisp/memory-report.el            | 16 +++++++++++++++-
 test/lisp/emacs-lisp/memory-report-tests.el |  8 ++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/memory-report.el b/lisp/emacs-lisp/memory-report.el
index aee2a0079c..cfa7010af4 100644
--- a/lisp/emacs-lisp/memory-report.el
+++ b/lisp/emacs-lisp/memory-report.el
@@ -29,7 +29,8 @@
 
 (require 'seq)
 (require 'subr-x)
-(eval-when-compile (require 'cl-lib))
+(require 'cl-lib)
+(require 'cl-macs)
 
 (defvar memory-report--type-size (make-hash-table))
 
@@ -243,6 +244,19 @@ memory-report--object-size-1
      value)
     total))
 
+;; for all cl-defstruct types
+(cl-defmethod memory-report--object-size-1 (counted (value cl-structure-object))
+  (let ((struct-type (type-of value)))
+    (apply '+
+           (memory-report--size 'vector)
+           (mapcar (lambda (slot)
+                     (if (eq (car slot) 'cl-tag-slot)
+                         0
+                       (memory-report--object-size
+                        counted
+                        (cl-struct-slot-value struct-type (car slot) value))))
+                   (cl-struct-slot-info struct-type)))))
+
 (defun memory-report--format (bytes)
   (setq bytes (/ bytes 1024.0))
   (let ((units '("KiB" "MiB" "GiB" "TiB")))
diff --git a/test/lisp/emacs-lisp/memory-report-tests.el b/test/lisp/emacs-lisp/memory-report-tests.el
index 0c0297b5fc..e352dd165f 100644
--- a/test/lisp/emacs-lisp/memory-report-tests.el
+++ b/test/lisp/emacs-lisp/memory-report-tests.el
@@ -68,6 +68,14 @@ memory-report-sizes-vectors
                 (vector string string))
                124))))
 
+(ert-deftest memory-report-sizes-structs ()
+  (cl-defstruct memory-report-test-struct
+    (item0 nil)
+    (item1 nil))
+  (let ((s (make-memory-report-test-struct :item0 "hello" :item1 "world")))
+    (should (= (memory-report-object-size s)
+               90))))
+
 (provide 'memory-report-tests)
 
 ;;; memory-report-tests.el ends here
-- 
2.33.0

Re: [PATCH] memory-report: support calculating size for structures

[Lars Ingebrigtsen]

"Yikai Zhao" <yikai@z1k.dev> writes:

> Here's another patch on memory-report package which fixes the object
> size calculation for structures (cl-defstruct types). Before this
> patch, memory-report--object-size will report empty size for structure
> objects, which results in incorrect report about "Largest Variables"
> in memory-report result.

Thanks; looks good.

> Regarding the copyright assignment, I'm currently in the progress of
> signing it. But maybe this patch is small enough to apply without it?

In itself, it would be small enough, but with the previous patch, it
pushes it above the limit.  So after the assignment process is complete,
can you remind us to apply the patch.  (And perhaps put the patch into
the issue tracker with `M-x submit-emacs-patch' so that it's not
forgotten.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: [PATCH] memory-report: support calculating size for structures

[Yikai Zhao]

On Mon, Aug 30, 2021, at 03:11, Lars Ingebrigtsen wrote:
> "Yikai Zhao" <yikai@z1k.dev> writes:
> 
> > Here's another patch on memory-report package which fixes the object
> > size calculation for structures (cl-defstruct types). Before this
> > patch, memory-report--object-size will report empty size for structure
> > objects, which results in incorrect report about "Largest Variables"
> > in memory-report result.
> 
> Thanks; looks good.
> 
> > Regarding the copyright assignment, I'm currently in the progress of
> > signing it. But maybe this patch is small enough to apply without it?
> 
> In itself, it would be small enough, but with the previous patch, it
> pushes it above the limit.  So after the assignment process is complete,
> can you remind us to apply the patch.  

I sent the signed document yesterday. I'm not sure if I should wait for the reply for the process to be complete.

> (And perhaps put the patch into
> the issue tracker with `M-x submit-emacs-patch' so that it's not
> forgotten.)

Aha, I didn't know that. I just sent a copy using `submit-emacs-patch'.

Thanks!

Yikai

Re: [PATCH] memory-report: support calculating size for structures

[Eli Zaretskii]

> Date: Wed, 01 Sep 2021 00:15:22 +0800
> From: "Yikai Zhao" <yikai@z1k.dev>
> Cc: emacs-devel@gnu.org
> 
> > > Regarding the copyright assignment, I'm currently in the progress of
> > > signing it. But maybe this patch is small enough to apply without it?
> > 
> > In itself, it would be small enough, but with the previous patch, it
> > pushes it above the limit.  So after the assignment process is complete,
> > can you remind us to apply the patch.  
> 
> I sent the signed document yesterday. I'm not sure if I should wait for the reply for the process to be complete.

Yes, we need to wait for the process to complete.
Thanks.

Re: [PATCH] memory-report: support calculating size for structures

[Yikai Zhao]

On Wed, Sep 1, 2021, at 00:27, Eli Zaretskii wrote:
> > Date: Wed, 01 Sep 2021 00:15:22 +0800
> > From: "Yikai Zhao" <yikai@z1k.dev>
> > Cc: emacs-devel@gnu.org
> > 
> > > > Regarding the copyright assignment, I'm currently in the progress of
> > > > signing it. But maybe this patch is small enough to apply without it?
> > > 
> > > In itself, it would be small enough, but with the previous patch, it
> > > pushes it above the limit.  So after the assignment process is complete,
> > > can you remind us to apply the patch.  
> > 
> > I sent the signed document yesterday. I'm not sure if I should wait for the reply for the process to be complete.
> 
> Yes, we need to wait for the process to complete.
> Thanks.
> 

The process is now complete. Please handle the patch at your convenience, Thanks!