Path to enable a module to change the stack_base_ptr

Started by Thomas Hallgrenalmost 21 years ago7 messagespatches
Jump to latest
#1Thomas Hallgren
thhal@mailblocks.com

Here is a patch that will enable a module to change the stack_base_ptr
temporarilly during a call. A background discussion can be found here:

http://www.mail-archive.com/pgsql-hackers@postgresql.org/msg64586.html

Regards,
Thomas Hallgren

Attachments:

stackbase.difftext/plain; name=stackbase.diffDownload+22-0
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Thomas Hallgren (#1)
Re: Path to enable a module to change the stack_base_ptr

Thomas Hallgren <thhal@mailblocks.com> writes:

Here is a patch that will enable a module to change the stack_base_ptr
temporarilly during a call.

I'm not really in favor of this ... I think you are trying to make the
backend do something that will never work reliably.

If we were to try to support this, I'd prefer to just make
stack_base_ptr non static, rather than add overhead code.

regards, tom lane

#3Thomas Hallgren
thhal@mailblocks.com
In reply to: Tom Lane (#2)
Re: Path to enable a module to change the stack_base_ptr

Tom Lane wrote:

Thomas Hallgren <thhal@mailblocks.com> writes:

Here is a patch that will enable a module to change the stack_base_ptr
temporarilly during a call.

I'm not really in favor of this ... I think you are trying to make the
backend do something that will never work reliably.

If we were to try to support this, I'd prefer to just make
stack_base_ptr non static, rather than add overhead code.

regards, tom lane

Sure, that works for me.

Regards,
Thomas Hallgren

#4Bruce Momjian
bruce@momjian.us
In reply to: Thomas Hallgren (#3)
Re: Path to enable a module to change the stack_base_ptr

Thomas Hallgren wrote:

Tom Lane wrote:

Thomas Hallgren <thhal@mailblocks.com> writes:

Here is a patch that will enable a module to change the stack_base_ptr
temporarilly during a call.

I'm not really in favor of this ... I think you are trying to make the
backend do something that will never work reliably.

If we were to try to support this, I'd prefer to just make
stack_base_ptr non static, rather than add overhead code.

regards, tom lane

Sure, that works for me.

Do we want to make this change for 8.1?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#4)
Re: Path to enable a module to change the stack_base_ptr

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Tom Lane wrote:

I'm not really in favor of this ... I think you are trying to make the
backend do something that will never work reliably.

Do we want to make this change for 8.1?

I don't want to do it at all. The justification given is to allow the
backend to support multithreading introduced by an add-on library, which
is a hopeless cause. Removing "static" from that variable declaration
is surely a cheap enough change, but what about the next request, and
the one after that?

regards, tom lane

#6Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#5)
Re: Path to enable a module to change the stack_base_ptr

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Tom Lane wrote:

I'm not really in favor of this ... I think you are trying to make the
backend do something that will never work reliably.

Do we want to make this change for 8.1?

I don't want to do it at all. The justification given is to allow the
backend to support multithreading introduced by an add-on library, which
is a hopeless cause. Removing "static" from that variable declaration
is surely a cheap enough change, but what about the next request, and
the one after that?

Well, I have not seen the next request yet, but it seems harmless for a
useful extension to the database, namely PL/Java. I do believe this is
one of the reasons PL/J took a different approach, though.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#7Thomas Hallgren
thhal@mailblocks.com
In reply to: Tom Lane (#5)
Re: Path to enable a module to change the stack_base_ptr

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Tom Lane wrote:

I'm not really in favor of this ... I think you are trying to make the
backend do something that will never work reliably.

Do we want to make this change for 8.1?

I don't want to do it at all. The justification given is to allow the
backend to support multithreading introduced by an add-on library, which
is a hopeless cause. Removing "static" from that variable declaration
is surely a cheap enough change, but what about the next request, and
the one after that?

Tom, I don't request that the backend should support multiple threads
simultaneously. It's one thread at a time. I can't think of a "next
request" in this direction. I'm very aware that the backend is
single-threaded and that you have no intention to change that. Neither do I.

Having the stack_base public is actually useful for another purpose
also. It can allow you to make assertions that check if an abitrary
pointer is 'on stack' or not. The MemoryContextContains() could be made
safer too by just returning false when the given pointer is between the
stack_base and the current stack_pointer. Perhaps that could be added to
the patch?

Regards,
Thomas Hallgren