all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Andrew Cohen <acohen@ust.hk>
To: "Eli Zaretskii" <eliz@gnu.org>,
	"Mattias Engdegård" <mattiase@acm.org>, larsi <larsi@gnus.org>
Cc: 54532@debbugs.gnu.org
Subject: bug#54532: [PATCH] sorting
Date: Thu, 24 Mar 2022 07:43:03 +0800	[thread overview]
Message-ID: <87wngkfd6g.fsf@ust.hk> (raw)
In-Reply-To: <83ee2seqyj.fsf@gnu.org> (Eli Zaretskii's message of "Wed, 23 Mar 2022 09:30:56 -0400")

>>>>> "EZ" == Eli Zaretskii <eliz@gnu.org> writes:

    >> From: Andrew Cohen <acohen@ust.hk> Date: Wed, 23 Mar 2022
    >> 07:59:11 +0800
    >> 
    >> 1. Add a new `record_unwind_protect_ptr_mark` function for use
    >> with C data structures that use the specpdl for clean-up but also
    >> contain possibly unique references to Lisp objects. This is
    >> needed for the dynamic memory management that the new algorithm
    >> uses.

    EZ> Can you tell more why this was needed?  Emacs has conservative
    EZ> stack marking, so any Lisp object that is referred by some
    EZ> stack-based variable should be protected from GC.  Why isn't
    EZ> that enough in this case?

Mattias responded, but just to recap---during the merging of sublists
additional memory is dynamically xalloc'ed (and later xfree'd)  on the
heap, and there can be brief periods where the original Lisp_Objects
exist /only/ in the heap. The alternative (which we mentioned briefly in
earlier discussions) would be to allocate the maximum amount of needed
memory at the outset with SAFE_ALLOCA_LISP. This is what the current
sorting routine does. But for many (maybe most?) actual calls to timsort no
additional memory is needed so the dynamic allocation makes
significantly better use of resources. Mattias explained the issue to me
and once I understood it found it was easy to trigger; there is a
specific test in the unit test patch to check for this issue.

    >> 4. An optimization that resolves the sorting comparison symbol
    >> into the corresponding function before starting the sort.

    EZ> Any reasons this is a separate patch?

Just to make it easier to look at the rather lengthy code by segregating
things that are self-contained.  If/when we decide to put this in I
would probably squash down to two (or maybe 3) commits.

Thanks for the thorough look at the code, I continue to learn a lot!

Best,
Andy
-- 
Andrew Cohen





  reply	other threads:[~2022-03-23 23:43 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-22 23:59 bug#54532: [PATCH] sorting Andrew Cohen
2022-03-23 12:02 ` Lars Ingebrigtsen
2022-03-23 13:30 ` Eli Zaretskii
2022-03-23 23:43   ` Andrew Cohen [this message]
2022-03-23 13:46 ` Eli Zaretskii
2022-03-23 23:31   ` Andrew Cohen
2022-03-23 20:24 ` Mattias Engdegård
2022-03-24  6:42   ` Eli Zaretskii
2022-03-24  7:22     ` Andrew Cohen
2022-03-24  8:55       ` Eli Zaretskii
2022-03-24  9:17         ` Andrew Cohen
2022-03-24  9:55           ` Mattias Engdegård
2022-03-24  9:36     ` Mattias Engdegård
2022-03-31 12:03 ` Lars Ingebrigtsen
2022-03-31 13:58   ` Eli Zaretskii
2022-03-31 23:47     ` Andrew Cohen
2022-04-01  6:26       ` Eli Zaretskii
2022-06-07  7:06         ` Stefan Kangas
     [not found]           ` <877d5tgd11.fsf@ust.hk>
2022-06-07  9:07             ` Stefan Kangas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87wngkfd6g.fsf@ust.hk \
    --to=acohen@ust.hk \
    --cc=54532@debbugs.gnu.org \
    --cc=eliz@gnu.org \
    --cc=larsi@gnus.org \
    --cc=mattiase@acm.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.