Tarantool development patches archive
 help / color / mirror / Atom feed
From: Sergey Ostanevich via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Igor Munkin <imun@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH luajit v1] tools: fix luajit-gdb stack dump
Date: Thu, 22 Jul 2021 15:51:55 +0300
Message-ID: <08A03966-6DEA-4AE6-B962-0C3B2D436BFA@tarantool.org> (raw)
In-Reply-To: <20210722114821.GM11494@tarantool.org>

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

Igor,

Sure you can proceed with the push.

Sergos.

> On 22 Jul 2021, at 14:48, Igor Munkin <imun@tarantool.org> wrote:
> 
> Sergos,
> 
> Thanks for your review! TL;DR: I would rather leave the patch with no
> changes, but please consider my comments regarding your alternative
> approach for iterating through the guest stack.
> 
> On 22.07.21, Sergey Ostanevich wrote:
>> Hi!
>> 
>> Thanks for the patch!
>> 
>> I would rather remove all mentions of ‘bottom loop’ as we named it at
>> Intel. I'm not a big pro in python, still the ‘for’ loop works fine 
>> for me:
>> 
>> If we can unwind to the next frame - dump it (and further)
>> 
>>>>> s=[1]
>>>>> for i in s:
>> ...   if (i==1):
>> ...     s.append(2)
>> ...     continue
>> ...   print(i)
>> ... 
>> 2
>> 
>> If it is the only one (dummy):
>> 
>>>>> s=[3]
>>>>> for i in s:
>> ...   if (i==1):
>> ...     s.append(2)
>> ...     continue
>> ...   print(i)
>> ... 
>> 3
>> 
>> Does it make sense in this case?
> 
> Let's see what would we get for our case with guest stack unwinding. The
> algorithm is described by Sergey in the comment near the loop. But
> imagine, we are iterating (backwards) through [L->stack, L-top] segment
> via for loop (as you suggest above). There is one condition to check
> whether we have reached the bottom of the frame (dump framelink slot and
> continue the same way you showed above). However, we need one more
> condition to skip the second slot for the framelink in LJ_FR2 mode.  As
> a result we have a loop that iterates through the stack with at least
> two conditions to skip some slots or dump them in other way.
> Furthermore, your proposal leads to much more complex change in the
> extension, comparing to those made by Sergey.
> 
> As I've already said, postcondition loop is the classic way for this
> case. Moreover, I googled "how to emulate a do-while loop in Python?"
> and the second option is unrolling the first iteration[1]. If one
> wonders what is the first option -- use "while-true" loop with the
> conditional break at the end. I equally hate both, so I'm OK with the
> one chosen by Sergey. If you prefer "while-true", I believe Sergey can
> rewrite this part, but considering your LGTM below, I guess you're OK
> also with the current solution. So, I'll push the patch in a jiffy.
> 
>> 
>> Anyways, it’s too much of nitpicking - LGTM in its current version.
>> 
>> Regards,
>> Sergos
>> 
>> 
> 
> <snipped>
> 
>> 
> 
> [1]: https://stackoverflow.com/a/743186 <https://stackoverflow.com/a/743186>
> 
> -- 
> Best regards,
> IM


[-- Attachment #2: Type: text/html, Size: 29396 bytes --]

  reply	other threads:[~2021-07-22 12:51 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-18  6:22 Sergey Kaplun via Tarantool-patches
2021-07-05  7:03 ` Sergey Kaplun via Tarantool-patches
2021-07-20 12:29 ` Igor Munkin via Tarantool-patches
2021-07-21  8:49   ` Sergey Kaplun via Tarantool-patches
2021-07-21  8:54     ` Igor Munkin via Tarantool-patches
2021-07-22 10:32       ` Sergey Ostanevich via Tarantool-patches
2021-07-22 11:48         ` Igor Munkin via Tarantool-patches
2021-07-22 12:51           ` Sergey Ostanevich via Tarantool-patches [this message]
2021-07-21  9:02     ` Sergey Kaplun via Tarantool-patches
2021-07-22 13:34 ` Igor Munkin via Tarantool-patches

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=08A03966-6DEA-4AE6-B962-0C3B2D436BFA@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=imun@tarantool.org \
    --cc=sergos@tarantool.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

Tarantool development patches archive

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://lists.tarantool.org/tarantool-patches/0 tarantool-patches/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 tarantool-patches tarantool-patches/ https://lists.tarantool.org/tarantool-patches \
		tarantool-patches@dev.tarantool.org.
	public-inbox-index tarantool-patches

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git