Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#704 closed defect (fixed)

eql-specializer-objects are not freed (remove-method) — at Version 2

Reported by: lnostdal Owned by:
Priority: normal Milestone:
Component: Runtime (threads, GC) Version: trunk
Keywords: Cc:

Description (last modified by gb)

Just tried this in CCL and got the same result after a while.

Here is the code:

(defgeneric blah (x))

(defun test ()
  (let ((object (make-list 10000)))
    (defmethod blah ((x (eql object)))
      (format t "~A~%" x))
    (remove-method #'blah (first (generic-function-methods #'blah))))

Then do:

(loop (test))

..and memory use keeps rising.

Change History (2)

comment:1 Changed 10 years ago by gb

  • Resolution set to fixed
  • Status changed from new to closed

(In [13980]) Make the hash table used by INTERN-EQL-SPECIALIZER weak on value. Don't call RECORD-SOURCE-FILE in ENSURE-METHOD; do call it from the expansion of DEFMETHOD. (Could do so more concisely, but that involves a little bit of bootstrapping.) Fixes ticket:704.

comment:2 Changed 10 years ago by gb

  • Description modified (diff)

The mapping from objects to EQL specializers on those objects should be weak (if the specializer isn't otherwise referenced, it and the corresponding object can be GCed.)

There's another issue here. A defining macro like DEFMETHOD can expand into code which has arbitrary implementation-dependent execution-time effects (keeping track of source information or other debugging info) as well as the defined effects (ENSURE-METHOD or something equivalent, etc.) There's at least some argument that says that the benefits of retaining debugging information about otherwise unreferenced obsolete methods (and having that retention prevent the methods and/or their specializers from being GCed) outweigh the disadvantages of doing so. (I'm not sure that I believe that; I don't have too much good to say about how CCL handles source-location recording, but there's at least an argument there.)

Rather than (ab)using a defining-macro (DEFMETHOD) to do method redefinition declaratively at runtime, it should be possible to do this procedurally (via ENSURE-METHOD) and not worry about those other effects. It wasn't possible prior to r13980 in the trunk; ENSURE-METHOD was calling CCL::RECORD-SOURCE-FILE, and that should really happen elsewhere in the expansion of DEFMETHOD.

In other words, if your example used ENSURE-METHOD, I think that its memory utilization should be relatively stable and AFAICT it is in the CCL trunk as of r13980.) If the example uses DEFMETHOD, it's not totally unreasonable that environmental/debugging info accrues, and that information costs something; someone who finds that information more useful than I do might consider that to be a reasonable tradeoff in many cases.

Note: See TracTickets for help on using tickets.