From: William S F. <ws...@fu...> - 2013-09-19 07:20:41
|
I'm looking for branches to merge onto master now. Robert, it looks like the talby-perl5-improvements branch might be ready? Can you confirm, if so, I'll start thinking about merging it in. William |
From: Robert S. <ta...@tr...> - 2013-11-13 22:46:06
|
I have spilled some work to https://github.com/talby-/swig/tree/perl5-directors-minimal representing a raw translation of the python director system to perl5. I believe this attempt should be fully reverse compatible with existing perl code and fairly cross compatible with python directors. I have included documentation updates, test-suite entries for many of the director tests, and usage examples. I still have to work out a destuctor problem, make code style match the existing codebase, do profiling & coverage checks, and verify portability against perl versions other than v5.10. Before I put much more time into this path, I'd like to know if it appears to be a sensible approach. -Robert |
From: William S F. <ws...@fu...> - 2013-11-16 09:35:56
|
On 13 November 2013 22:45, Robert Stone <ta...@tr...> wrote: > I have spilled some work to > https://github.com/talby-/swig/tree/perl5-directors-minimal representing > a raw translation of the python director system to perl5. I believe > this attempt should be fully reverse compatible with existing perl code > and fairly cross compatible with python directors. > > I have included documentation updates, test-suite entries for > many of the director tests, and usage examples. I still have to work > out a destuctor problem, make code style match the existing codebase, do > profiling & coverage checks, and verify portability against perl > versions other than v5.10. > > Before I put much more time into this path, I'd like to know if > it appears to be a sensible approach. > > > Hi Robert Excellent, I can't see anything wrong with this approach. I was thinking the Perl changes in your talby-perl5-improvements branch is similar to Python's -builtin option where the Proxy class definitions are moved from pure Perl/Python code to C. In Python, the builtin approach is better all round, but not so 100% compatible with existing code (although close). This should sound familiar! Perhaps you would like to bring in the changes from that branch and have it turned on via a similar command line switch to -builtin? That way users get to choose. Hopefully you sort out the destructor problem, but if it is not solvable, how about making the directors available only with the -builtin equivalent approach? As it is SWIG 3 and a chance to make fundamental changes, we could even turn the default code generation to be the builtin, with a -nobuiltin / -legacy switch for old code generation which will still work with -noproxy. Given how much work you've done and the improvements you have done, I presume that the various ways to generate code can be combined and incorporated together with command line switches to enable. Does that sound right? William |
From: Robert S. <ta...@tr...> - 2013-11-17 19:49:24
|
On Sat, Nov 16, 2013 at 09:26:55AM +0000, William S Fulton wrote: > I was thinking the Perl changes in your talby-perl5-improvements > branch is similar to Python's -builtin option where the Proxy class > definitions are moved from pure Perl/Python code to C. In Python, the > builtin approach is better all round, but not so 100% compatible with > existing code (although close). This should sound familiar! Perhaps > you would like to bring in the changes from that branch and have it > turned on via a similar command line switch to -builtin? That way > users get to choose. That sounds like a good path. I'll investigate how this might work out, but I don't think I'll be able to do this rapidly. > Hopefully you sort out the destructor problem, but if it is not > solvable, how about making the directors available only with the > -builtin equivalent approach? As it is SWIG 3 and a chance to make > fundamental changes, we could even turn the default code generation to > be the builtin, with a -nobuiltin / -legacy switch for old code > generation which will still work with -noproxy. Given how much work > you've done and the improvements you have done, I presume that the > various ways to generate code can be combined and incorporated > together with command line switches to enable. Does that sound right? The destructor challenge is that in Perl when a SWIG wrapped object falls out of scope the "DESTROY()" method gets fired twice implicitly. When the caller requests object release, they call "DESTROY()" as well so it will get hit three times. Altering a destructor has always been a little tricky with SWIG in Perl because you must make your destructor idempotent or cope with it sometimes firing on what represents a dangling pointer. The problem didn't come up often in practice, but introducing directors will make custom destructors a lot more common. To ease this problem ~SwigDirector_Object can dispatch a method into Perl to signal the last time a director object will be live before teardown. I have coded that to make yet another call to the "DESTROY()" method, but send an extra argument to the call. You can see what that looks like from the users perspective in the director_finalizer run test at https://github.com/talby-/swig/blob/perl5-directors-minimal/Examples/test-suite/perl5/director_finalizer_runme.pl I hope this is a relatively safe thing to do from a reverse compatibility perspective since any SWIG users have already had to code around multiple destructor calls, and this is just a potential fourth type of destructor invocation. Alternatively, we could have ~SwigDirector_Object issue a new method call, "DESTROY_swig_director()" for example. That method would not need to be idempotent, but would run a bit slower and be another unique semantic not shared by either other Perl objects or other SWIG languages. -Robert |
From: William S F. <ws...@fu...> - 2013-12-18 00:01:19
|
On 17/11/13 19:49, Robert Stone wrote: > On Sat, Nov 16, 2013 at 09:26:55AM +0000, William S Fulton wrote: >> I was thinking the Perl changes in your talby-perl5-improvements >> branch is similar to Python's -builtin option where the Proxy class >> definitions are moved from pure Perl/Python code to C. In Python, the >> builtin approach is better all round, but not so 100% compatible with >> existing code (although close). This should sound familiar! Perhaps >> you would like to bring in the changes from that branch and have it >> turned on via a similar command line switch to -builtin? That way >> users get to choose. > > That sounds like a good path. I'll investigate how this might > work out, but I don't think I'll be able to do this rapidly. > Would you like to say how long this might take? Given all the good things your new implementation has, it might be worth making this the default. I ask as I see SWIG 3 as one golden opportunity to put in backwards breaking default behaviour so long as a command line switch will restore the old behaviour. >> Hopefully you sort out the destructor problem, but if it is not >> solvable, how about making the directors available only with the >> -builtin equivalent approach? As it is SWIG 3 and a chance to make >> fundamental changes, we could even turn the default code generation to >> be the builtin, with a -nobuiltin / -legacy switch for old code >> generation which will still work with -noproxy. Given how much work >> you've done and the improvements you have done, I presume that the >> various ways to generate code can be combined and incorporated >> together with command line switches to enable. Does that sound right? > > The destructor challenge is that in Perl when a SWIG wrapped > object falls out of scope the "DESTROY()" method gets fired twice > implicitly. When the caller requests object release, they call > "DESTROY()" as well so it will get hit three times. Altering a > destructor has always been a little tricky with SWIG in Perl because you > must make your destructor idempotent or cope with it sometimes firing on > what represents a dangling pointer. > > The problem didn't come up often in practice, but introducing > directors will make custom destructors a lot more common. To ease this > problem ~SwigDirector_Object can dispatch a method into Perl to signal > the last time a director object will be live before teardown. I have > coded that to make yet another call to the "DESTROY()" method, but send > an extra argument to the call. You can see what that looks like from > the users perspective in the director_finalizer run test at > https://github.com/talby-/swig/blob/perl5-directors-minimal/Examples/test-suite/perl5/director_finalizer_runme.pl > > I hope this is a relatively safe thing to do from a reverse > compatibility perspective since any SWIG users have already had to code > around multiple destructor calls, and this is just a potential fourth > type of destructor invocation. Alternatively, we could have > ~SwigDirector_Object issue a new method call, "DESTROY_swig_director()" > for example. That method would not need to be idempotent, but would run > a bit slower and be another unique semantic not shared by either other > Perl objects or other SWIG languages. I don't know that I can offer much on the Perl side. Do you think github patch #106 should be merged now? I'm thinking that if SWIG 3 can contain your new wrapping approach as the default alongside the old implementation restored with a command line option, I don't see much point in having to support directors in the old wrappers if directors are better supported in the new wrappers. William |
From: Robert S. <ta...@tr...> - 2013-12-18 02:07:02
|
On Wed, Dec 18, 2013 at 12:01:05AM +0000, William S Fulton wrote: > On 17/11/13 19:49, Robert Stone wrote: > >On Sat, Nov 16, 2013 at 09:26:55AM +0000, William S Fulton wrote: > >>I was thinking the Perl changes in your talby-perl5-improvements > >>branch is similar to Python's -builtin option where the Proxy class > >>definitions are moved from pure Perl/Python code to C. In Python, the > >>builtin approach is better all round, but not so 100% compatible with > >>existing code (although close). This should sound familiar! Perhaps > >>you would like to bring in the changes from that branch and have it > >>turned on via a similar command line switch to -builtin? That way > >>users get to choose. > > > > That sounds like a good path. I'll investigate how this might > >work out, but I don't think I'll be able to do this rapidly. > > > Would you like to say how long this might take? Given all the good > things your new implementation has, it might be worth making this > the default. I ask as I see SWIG 3 as one golden opportunity to put > in backwards breaking default behaviour so long as a command line > switch will restore the old behaviour. I've begun attempting a "-builtin" feature on top of the "perl5-directors-minimal" branch. It is going to be more complicated than reformatting the "talby-perl5-improvements" branch into conditional blocks. Allowing both wrapper types to coexist for imports.multicpptest style object sharing will require something more sophisticated than I have now. I expect it will take at least a couple of months to complete. I would not want something of that size to hold up a SWIG 3 release. I think it makes the most sense to phase that feature in more gradually. > > I hope this is a relatively safe thing to do from a reverse > >compatibility perspective since any SWIG users have already had to code > >around multiple destructor calls, and this is just a potential fourth > >type of destructor invocation. Alternatively, we could have > >~SwigDirector_Object issue a new method call, "DESTROY_swig_director()" > >for example. That method would not need to be idempotent, but would run > >a bit slower and be another unique semantic not shared by either other > >Perl objects or other SWIG languages. > > I don't know that I can offer much on the Perl side. Do you think > github patch #106 should be merged now? I'm thinking that if SWIG 3 > can contain your new wrapping approach as the default alongside the > old implementation restored with a command line option, I don't see > much point in having to support directors in the old wrappers if > directors are better supported in the new wrappers. If you don't have strong feelings one way or another about this, I think reusing the existing destructor is the better of the two unfortunate options. That is what's implemented in the pull request. I don't think implementing "-builtin" gets significantly easier if directors are not supported without it. It would give us a way out of this destructor reverse compatibility corner, but that's the only hard problem I think it would solve. -Robert |
From: William S F. <ws...@fu...> - 2013-12-19 07:05:17
|
On 18/12/13 02:06, Robert Stone wrote: > On Wed, Dec 18, 2013 at 12:01:05AM +0000, William S Fulton wrote: >> On 17/11/13 19:49, Robert Stone wrote: >>> On Sat, Nov 16, 2013 at 09:26:55AM +0000, William S Fulton wrote: >>>> I was thinking the Perl changes in your talby-perl5-improvements >>>> branch is similar to Python's -builtin option where the Proxy class >>>> definitions are moved from pure Perl/Python code to C. In Python, the >>>> builtin approach is better all round, but not so 100% compatible with >>>> existing code (although close). This should sound familiar! Perhaps >>>> you would like to bring in the changes from that branch and have it >>>> turned on via a similar command line switch to -builtin? That way >>>> users get to choose. >>> >>> That sounds like a good path. I'll investigate how this might >>> work out, but I don't think I'll be able to do this rapidly. >>> >> Would you like to say how long this might take? Given all the good >> things your new implementation has, it might be worth making this >> the default. I ask as I see SWIG 3 as one golden opportunity to put >> in backwards breaking default behaviour so long as a command line >> switch will restore the old behaviour. > > I've begun attempting a "-builtin" feature on top of the > "perl5-directors-minimal" branch. It is going to be more complicated > than reformatting the "talby-perl5-improvements" branch into conditional > blocks. Allowing both wrapper types to coexist for imports.multicpptest > style object sharing will require something more sophisticated than I > have now. I expect it will take at least a couple of months to > complete. I would not want something of that size to hold up a SWIG 3 > release. I think it makes the most sense to phase that feature in > more gradually. It seems quite a reasonable restriction to me that %import should work only if you have the same style wrappers, that is, the -builtin equivalent usage is the same for each invocation of SWIG. By all means only add interopability later on if someone really needs it. Would that speed things up? > >>> I hope this is a relatively safe thing to do from a reverse >>> compatibility perspective since any SWIG users have already had to code >>> around multiple destructor calls, and this is just a potential fourth >>> type of destructor invocation. Alternatively, we could have >>> ~SwigDirector_Object issue a new method call, "DESTROY_swig_director()" >>> for example. That method would not need to be idempotent, but would run >>> a bit slower and be another unique semantic not shared by either other >>> Perl objects or other SWIG languages. >> >> I don't know that I can offer much on the Perl side. Do you think >> github patch #106 should be merged now? I'm thinking that if SWIG 3 >> can contain your new wrapping approach as the default alongside the >> old implementation restored with a command line option, I don't see >> much point in having to support directors in the old wrappers if >> directors are better supported in the new wrappers. > > If you don't have strong feelings one way or another about this, > I think reusing the existing destructor is the better of the two > unfortunate options. That is what's implemented in the pull request. > > I don't think implementing "-builtin" gets significantly easier > if directors are not supported without it. It would give us a way out > of this destructor reverse compatibility corner, but that's the only > hard problem I think it would solve. > Okay, sounds like that branch ought to be pulled in then. William |
From: Robert S. <ta...@tr...> - 2013-12-24 19:20:02
|
On Thu, Dec 19, 2013 at 07:05:00AM +0000, William S Fulton wrote: > On 18/12/13 02:06, Robert Stone wrote: > >On Wed, Dec 18, 2013 at 12:01:05AM +0000, William S Fulton wrote: > >>Would you like to say how long this might take? Given all the good > >>things your new implementation has, it might be worth making this > >>the default. I ask as I see SWIG 3 as one golden opportunity to put > >>in backwards breaking default behaviour so long as a command line > >>switch will restore the old behaviour. > > > > I've begun attempting a "-builtin" feature on top of the > >"perl5-directors-minimal" branch. It is going to be more complicated > >than reformatting the "talby-perl5-improvements" branch into conditional > >blocks. Allowing both wrapper types to coexist for imports.multicpptest > >style object sharing will require something more sophisticated than I > >have now. I expect it will take at least a couple of months to > >complete. I would not want something of that size to hold up a SWIG 3 > >release. I think it makes the most sense to phase that feature in > >more gradually. > It seems quite a reasonable restriction to me that %import should > work only if you have the same style wrappers, that is, the -builtin > equivalent usage is the same for each invocation of SWIG. By all > means only add interopability later on if someone really needs it. > Would that speed things up? That should make things easier. I will work through what it will look like, and get a branch published once I have some basics working. I'd like to retain runtime object sharing between modules with minimal restrictions, but I won't hold that as a core requirement. > Okay, sounds like that branch ought to be pulled in then. It will be nice to see Perl directors in the stable release. I'd been kicking that project around for far too long. -Robert |
From: William S F. <ws...@fu...> - 2013-12-26 11:20:26
|
On 24/12/13 19:19, Robert Stone wrote: > On Thu, Dec 19, 2013 at 07:05:00AM +0000, William S Fulton wrote: >> Okay, sounds like that branch ought to be pulled in then. > > It will be nice to see Perl directors in the stable release. > I'd been kicking that project around for far too long. I merged it in a few days ago, on behalf of all the Perl users, many thanks for this contribution. William |
From: Robert S. <ta...@tr...> - 2013-09-19 19:32:32
|
On Thu, Sep 19, 2013 at 08:20:28AM +0100, William S Fulton wrote: > I'm looking for branches to merge onto master now. Robert, it looks > like the talby-perl5-improvements branch might be ready? Can you > confirm, if so, I'll start thinking about merging it in. > I've completed the major documentation updates I mentioned last time. However, there are two more things I have planned. The first is to move and clean up the documentation from Lib/perl5/GUTS into the Doc/Manual/Perl5.html file. The second is to write an update for CHANGES.current. I can notify you when I've done those two things, but you can proceed without them if you like. I should have them soon. -Robert |
From: William S F. <ws...@fu...> - 2013-09-24 21:22:25
|
On 19/09/13 20:32, Robert Stone wrote: > On Thu, Sep 19, 2013 at 08:20:28AM +0100, William S Fulton wrote: >> I'm looking for branches to merge onto master now. Robert, it looks >> like the talby-perl5-improvements branch might be ready? Can you >> confirm, if so, I'll start thinking about merging it in. >> > > I've completed the major documentation updates I mentioned last > time. However, there are two more things I have planned. The first is > to move and clean up the documentation from Lib/perl5/GUTS into the > Doc/Manual/Perl5.html file. The second is to write an update for > CHANGES.current. > > I can notify you when I've done those two things, but you can > proceed without them if you like. I should have them soon. I committed a couple of minor tweaks on the branch and there are some further small improvements to do before the merge. A couple of observations in perlrun.swg: +#define SWIG_POINTER_MG 0x4 +#define SWIG_POINTER_WR 0x8 +/* These Perl specific pointer conversion flags handle two alternate + * types of native pointer recovery, _MG handles wrapped variables and + * object attributes, and _WR handles destructors. + * + * TODO: document the shadow object memory layout. + */ What do _MG and _WR stand for? Perhaps a longer name would be better or document them here. -#define SWIG_SHADOW SWIG_OWNER << 1 I was wondering if this would break current code and if there is a backwards compatible replacement? I don't think this was documented, however, a quick bit of googling showed up some use of it. Hopefully and preferably you can restore this macro with something that works as it used to, otherwise at the minimum, can you put something in the changes file as to how to code up a replacement. director_ignore.i shows a problem as there are warnings on ignored methods. The _wrap.h file for directors is missing. The contents seemed to have ended up in the _wrap.cxx file. Could you also convert to perl and add the 'extend' and 'callback' examples? These are the standard examples showing off directors. A few more lines of documentation in the director section introducing what they do would be nice. I don't think anyone reading it will fully realise the power of directors and that the feature enables cross language polymorphism and callbacks. The Python chapter on the subject is probably worth cribbing. I see cwrap.c has new attribute handling for "arg:classref" and I'm trying to work out why this is necessary given that this additional 'self' parameter is also in the other director supported languages. It looks like these are equivalent to Python's 'PyObject *self' and Ruby's 'VALUE self', Java's 'jobject jself' etc. It seems that you are bypassing the 'use_director' in Swig_ConstructorToFunction. Also can the name be changed to 'SV self' or does 'SV proto' make a lot more sense in Perl? William |
From: Robert S. <ta...@tr...> - 2013-09-25 01:22:10
|
On Tue, Sep 24, 2013 at 10:22:15PM +0100, William S Fulton wrote: > On 19/09/13 20:32, Robert Stone wrote: > >On Thu, Sep 19, 2013 at 08:20:28AM +0100, William S Fulton wrote: > >>I'm looking for branches to merge onto master now. Robert, it looks > >>like the talby-perl5-improvements branch might be ready? Can you > >>confirm, if so, I'll start thinking about merging it in. > >> > > > > I've completed the major documentation updates I mentioned last > >time. However, there are two more things I have planned. The first is > >to move and clean up the documentation from Lib/perl5/GUTS into the > >Doc/Manual/Perl5.html file. The second is to write an update for > >CHANGES.current. > > > > I can notify you when I've done those two things, but you can > >proceed without them if you like. I should have them soon. > > I committed a couple of minor tweaks on the branch and there are > some further small improvements to do before the merge. > > A couple of observations in perlrun.swg: > > +#define SWIG_POINTER_MG 0x4 > +#define SWIG_POINTER_WR 0x8 > +/* These Perl specific pointer conversion flags handle two alternate > + * types of native pointer recovery, _MG handles wrapped variables and > + * object attributes, and _WR handles destructors. > + * > + * TODO: document the shadow object memory layout. > + */ > > What do _MG and _WR stand for? Perhaps a longer name would be better > or document them here. > > -#define SWIG_SHADOW SWIG_OWNER << 1 > I was wondering if this would break current code and if there is a > backwards compatible replacement? I don't think this was documented, > however, a quick bit of googling showed up some use of it. Hopefully > and preferably you can restore this macro with something that works > as it used to, otherwise at the minimum, can you put something in > the changes file as to how to code up a replacement. > > director_ignore.i shows a problem as there are warnings on ignored methods. > > The _wrap.h file for directors is missing. The contents seemed to > have ended up in the _wrap.cxx file. > > Could you also convert to perl and add the 'extend' and 'callback' > examples? These are the standard examples showing off directors. > > A few more lines of documentation in the director section > introducing what they do would be nice. I don't think anyone reading > it will fully realise the power of directors and that the feature > enables cross language polymorphism and callbacks. The Python > chapter on the subject is probably worth cribbing. > > I see cwrap.c has new attribute handling for "arg:classref" and I'm > trying to work out why this is necessary given that this additional > 'self' parameter is also in the other director supported languages. > It looks like these are equivalent to Python's 'PyObject *self' and > Ruby's 'VALUE self', Java's 'jobject jself' etc. It seems that you > are bypassing the 'use_director' in Swig_ConstructorToFunction. > > Also can the name be changed to 'SV self' or does 'SV proto' make a > lot more sense in Perl? I'll start on these updates this evening. Thanks for looking over this work! -Robert |
From: William S F. <ws...@fu...> - 2013-10-23 17:08:04
|
On 22/10/13 22:45, Robert Stone wrote: > On Tue, Oct 22, 2013 at 07:21:02PM +0100, William S Fulton wrote: >> It is possible to add in extra commandline options per testcase. >> Examples/test-suite/csharp/Makefile has such an example: >> >> # Custom tests - tests with additional commandline options >> intermediary_classname.cpptest: SWIGOPT += -dllimport intermediary_classname >> >> and for all languages, see common.mk, eg: >> # Custom tests - tests with additional commandline options >> wallkw.cpptest: SWIGOPT += -Wallkw >> preproc_include.ctest: SWIGOPT += -includeall >> >> By all means add in one or two non-standard commandline tests, but >> really most of the test-suite should run using the default options. > > That's great, thank you for the hints. > >>> I will try to work on this further. It probably makes sense to >>> investigate not trying to unify the director proxy system with the >>> existing one. If I can do that, retaining complete reverse >>> compatibility should be trivial. >> What do you mean by this? There is no director support in Perl on >> master. Perhaps you don't want to use all the common director >> support code? If so, I'd really want to understand why and look at >> ways to keep the code common. > > I mean that I think I can significantly reduce the risk of this > feature by re-cutting my work as a set of enhancements that only affect > wrappers when directors are enabled. I have been trying to deeply > change the way SWIG handles Perl, but I think this might be a mistake. > I have not been thinking about "-noproxy" mode much and I expect to find > other compatibility problems. Can't you just deal with -noproxy with "if (!blessed) ..." in perl.cxx? That is pretty much how we cope with it in other languages. The idea is that a flattened C interface is generated and then the proxy classes are object oriented wrappers around it. Even though you have moved the Perl proxy classes from being defined in Perl code to C++ code, surely this concept still applies? > > Mechanically, this would be trying to change the proposal to a > set of additions to the code wrapped in "if (directorsEnabled()) {}" > conditions as much as possible. That would to make the static member > function compatibility issue vanish, and solve the legacy destructor > problem. It should also mean the code can continue to support perl v5.6 > when directors are not needed. > What minimum version is required for your branch as it stands? Or in other words, what minimum version is required to support directors if taking the new approach in the above paragraph? > One downside would be that SWIG_GetModule() wouldn't be able to > share objects between director enabled and directorless modules. And, > of course, enabling directors for existing code would trigger the > altered static member function and destructor semantics. > What if someone turns on directors for a module. Will a user's existing Perl code continue to work unaltered? In particular will the creation and deletion of objects continue to work? I presume that "delete obj" will work with and without this new approach? Unless I understand this incorrectly, this sounds messy, ie use the old wrapping approach with Perl code for proxy classes, then using C++ defined proxy classes for directors. It sounds too different when turning on directors when really all that should happen is everything is much the same except for polymorphic upcall support being added. Overall it looks like a hard call as I think you are saying that providing full backwards compatibility is hard in order to add director support. I don't object to breaking backwards compatibility if we really have to in order to take a few steps forwards. Clearly directors are a big step forward. Are there other improvements in the new proxy classes generation? William |
From: Robert S. <ta...@tr...> - 2013-10-24 23:01:36
|
On Wed, Oct 23, 2013 at 06:06:47PM +0100, William S Fulton wrote: > On 22/10/13 22:45, Robert Stone wrote: > > I mean that I think I can significantly reduce the risk of this > >feature by re-cutting my work as a set of enhancements that only affect > >wrappers when directors are enabled. I have been trying to deeply > >change the way SWIG handles Perl, but I think this might be a mistake. > >I have not been thinking about "-noproxy" mode much and I expect to find > >other compatibility problems. > Can't you just deal with -noproxy with "if (!blessed) ..." in > perl.cxx? That is pretty much how we cope with it in other > languages. The idea is that a flattened C interface is generated and > then the proxy classes are object oriented wrappers around it. Even > though you have moved the Perl proxy classes from being defined in > Perl code to C++ code, surely this concept still applies? The concept still applies. I should be able to bring it back. I thought the mode was deprecated and wasn't giving it a great deal of attention beyond the potential for crashes. > > Mechanically, this would be trying to change the proposal to a > >set of additions to the code wrapped in "if (directorsEnabled()) {}" > >conditions as much as possible. That would to make the static member > >function compatibility issue vanish, and solve the legacy destructor > >problem. It should also mean the code can continue to support perl v5.6 > >when directors are not needed. > > > What minimum version is required for your branch as it stands? Or in > other words, what minimum version is required to support directors > if taking the new approach in the above paragraph? The branch requires perl v5.8, but I should be able to retain v5.6 support back if I omit the new object wrappers. I forgot that master still runs on perl v5.6 until I was back reading the docs on the master branch about the explicit DESTROY invocation. > > One downside would be that SWIG_GetModule() wouldn't be able to > >share objects between director enabled and directorless modules. And, > >of course, enabling directors for existing code would trigger the > >altered static member function and destructor semantics. > > > What if someone turns on directors for a module. Will a user's > existing Perl code continue to work unaltered? In particular will > the creation and deletion of objects continue to work? I presume > that "delete obj" will work with and without this new approach? > Unless I understand this incorrectly, this sounds messy, ie use the > old wrapping approach with Perl code for proxy classes, then using > C++ defined proxy classes for directors. It sounds too different > when turning on directors when really all that should happen is > everything is much the same except for polymorphic upcall support > being added. This is correct, I was thinking that we might only use the new object wrappers and runtime semantics in director mode, but this would be a burden when directors are enabled on legacy code. If we omit the new object wrappers, then I suspect we could get to a point where full reverse compatibility is maintained. Directors would need SWIG specific mechanisms for subclassing, destructor methods, attribute declarations, and static member functions. I will investigate this route. > Overall it looks like a hard call as I think you are saying that > providing full backwards compatibility is hard in order to add > director support. I don't object to breaking backwards compatibility > if we really have to in order to take a few steps forwards. Clearly > directors are a big step forward. Are there other improvements in > the new proxy classes generation? I hope it doesn't seem like I'm being evasive. As you're asking me about these issues, I'm reevaluating if the costs are necessary. You reminded me of the hack I'd proposed in cwrap.c and I found a way around it that I never saw before. I think with more revelations like that one I can probably make directors work without an overhaul of the module. The main advantage of the branch is more performant, memory efficient, native Perl feeling objects that are less likely to trigger segfaults when used creatively. I'm really proud of what I've done there and have used it in production environments for a couple of large projects. The new object wrappers maintain a pointer cache and keep refcounts to better mimic the lifetimes of native Perl objects. This also allows %newobject and the DISOWN typemap to completely manage object ownership for nearly every use pattern I've encountered. The current perl5 wrapper runtime exposes all its guts to Perl, and consequently the wrapper code can do very little without trips into the interpreter. Something like: my $f = example::Foo->new(); $f->{i} = 12; my $g = example::Foo->new(); $g->{i} = 32; $g->DESTROY(); ${tied %$g} = ${tied %$f}; print $g->{i}, "\n"; $g->DISOWN(); actually works to make one wrapper alias another. That may seem like an abuse, but I wouldn't be surprised to see some variant of this used for array element access somewhere in the wild. Here are the benchmarks comparing master to the branch based on the current work: object creation: Rate trunk_new p5imp_new trunk_new 190934/s -- -48% p5imp_new 367991/s 93% -- attribute setter: Rate trunk_sa p5imp_sa trunk_sa 555474/s -- -93% p5imp_sa 8146970/s 1367% -- attribute getter: Rate trunk_ga p5imp_ga trunk_ga 543310/s -- -86% p5imp_ga 3814279/s 602% -- function call with native type arguments and return value: Rate trunk_nt p5imp_nt trunk_nt 5028510/s -- -4% p5imp_nt 5241261/s 4% -- function call with an object return value: Rate trunk_mkp p5imp_mkp trunk_mkp 380670/s -- -73% p5imp_mkp 1409188/s 270% -- function call with an object argument: Rate trunk_cvp p5imp_cvp trunk_cvp 4766252/s -- -14% p5imp_cvp 5534273/s 16% -- polymorphic function: Rate trunk_poly p5imp_poly trunk_poly 3260219/s -- -19% p5imp_poly 4034174/s 24% -- But I can't get these improvements in safety and performance without some dramatic retooling to lock control away from interpreter code. They ended up paired with the directors feature because it seemed necessary to get to directors at first, but now I'm not sure. |
From: Robert S. <ta...@tr...> - 2013-09-28 19:43:31
|
I have a couple of tasks remaining, but so far things are going smoothly. I've had to review some of my earliest work on this branch, and have found some success where I failed before. Thanks for the review. On Tue, Sep 24, 2013 at 10:22:15PM +0100, William S Fulton wrote: > > A couple of observations in perlrun.swg: > > +#define SWIG_POINTER_MG 0x4 > +#define SWIG_POINTER_WR 0x8 > +/* These Perl specific pointer conversion flags handle two alternate > + * types of native pointer recovery, _MG handles wrapped variables and > + * object attributes, and _WR handles destructors. > + * > + * TODO: document the shadow object memory layout. > + */ > > What do _MG and _WR stand for? Perhaps a longer name would be better > or document them here. I've renamed and clarified SWIG_POINTER_MG and removed the need for SWIG_POINTER_WR. I also moved the value out to 0x4000 to give swigrun.swg more room for additional flags in the future. > -#define SWIG_SHADOW SWIG_OWNER << 1 > I was wondering if this would break current code and if there is a > backwards compatible replacement? I don't think this was documented, > however, a quick bit of googling showed up some use of it. Hopefully > and preferably you can restore this macro with something that works > as it used to, otherwise at the minimum, can you put something in > the changes file as to how to code up a replacement. I purged this early on when removing "$shadow" at the request of a couple of you guys without really knowing what the feature did, but it was easy to bring SWIG_SHADOW back. > Could you also convert to perl and add the 'extend' and 'callback' > examples? These are the standard examples showing off directors. These are now in the branch. > I see cwrap.c has new attribute handling for "arg:classref" and I'm > trying to work out why this is necessary given that this additional > 'self' parameter is also in the other director supported languages. > It looks like these are equivalent to Python's 'PyObject *self' and > Ruby's 'VALUE self', Java's 'jobject jself' etc. It seems that you > are bypassing the 'use_director' in Swig_ConstructorToFunction. This was to handle what I think is somewhat unique to Perl, an implicit argument to static member functions and constructors. I was able to retool things to avoid the cwrap.c patches. > Also can the name be changed to 'SV self' or does 'SV proto' make a > lot more sense in Perl? This is a Perl idiom for the implicit first argument of class methods before the callee knows whether the argument is a class reference or an object instance. It would be misleading to name it "self", though I can understand your confusion. I expect as support for multiple inheritance evolves, the difference should become much clearer. However, with removing the cwrap.c patches, the only mention of "proto" is now in director classes. |
From: William S F. <ws...@fu...> - 2013-10-07 07:04:47
|
On 28/09/13 20:43, Robert Stone wrote: > I have a couple of tasks remaining, but so far things are going > smoothly. I've had to review some of my earliest work on this branch, > and have found some success where I failed before. Thanks for the > review. Thanks, looks like good progress. I'm keen to merge it across, so please let me know when you are done. William |
From: William S F. <ws...@fu...> - 2013-10-15 07:00:28
|
Is there much more to do or is it ready now for merging? On 28/09/13 20:43, Robert Stone wrote: > I have a couple of tasks remaining, but so far things are going > smoothly. I've had to review some of my earliest work on this branch, > and have found some success where I failed before. Thanks for the > review. > > On Tue, Sep 24, 2013 at 10:22:15PM +0100, William S Fulton wrote: >> >> A couple of observations in perlrun.swg: >> >> +#define SWIG_POINTER_MG 0x4 >> +#define SWIG_POINTER_WR 0x8 >> +/* These Perl specific pointer conversion flags handle two alternate >> + * types of native pointer recovery, _MG handles wrapped variables and >> + * object attributes, and _WR handles destructors. >> + * >> + * TODO: document the shadow object memory layout. >> + */ >> >> What do _MG and _WR stand for? Perhaps a longer name would be better >> or document them here. > > I've renamed and clarified SWIG_POINTER_MG and removed the need > for SWIG_POINTER_WR. I also moved the value out to 0x4000 to give > swigrun.swg more room for additional flags in the future. > >> -#define SWIG_SHADOW SWIG_OWNER << 1 >> I was wondering if this would break current code and if there is a >> backwards compatible replacement? I don't think this was documented, >> however, a quick bit of googling showed up some use of it. Hopefully >> and preferably you can restore this macro with something that works >> as it used to, otherwise at the minimum, can you put something in >> the changes file as to how to code up a replacement. > > I purged this early on when removing "$shadow" at the request of > a couple of you guys without really knowing what the feature did, but it > was easy to bring SWIG_SHADOW back. > >> Could you also convert to perl and add the 'extend' and 'callback' >> examples? These are the standard examples showing off directors. > > These are now in the branch. > >> I see cwrap.c has new attribute handling for "arg:classref" and I'm >> trying to work out why this is necessary given that this additional >> 'self' parameter is also in the other director supported languages. >> It looks like these are equivalent to Python's 'PyObject *self' and >> Ruby's 'VALUE self', Java's 'jobject jself' etc. It seems that you >> are bypassing the 'use_director' in Swig_ConstructorToFunction. > > This was to handle what I think is somewhat unique to Perl, an > implicit argument to static member functions and constructors. I was > able to retool things to avoid the cwrap.c patches. > >> Also can the name be changed to 'SV self' or does 'SV proto' make a >> lot more sense in Perl? > > This is a Perl idiom for the implicit first argument of class > methods before the callee knows whether the argument is a class > reference or an object instance. It would be misleading to name it > "self", though I can understand your confusion. I expect as support for > multiple inheritance evolves, the difference should become much clearer. > However, with removing the cwrap.c patches, the only mention of "proto" > is now in director classes. > |
From: Robert S. <ta...@tr...> - 2013-10-15 19:32:55
|
On Tue, Oct 15, 2013 at 07:31:56AM +0100, William S Fulton wrote: > Is there much more to do or is it ready now for merging? > There is not much remaining. I am working on expanding the Perl5 director docs, and that process has me looking at things pretty closely. I'm not confident about how director destructors are currently handled, but I should be able to clean it up in the next couple of days. -Robert |
From: Robert S. <ta...@tr...> - 2013-10-20 20:08:37
|
On Tue, Oct 15, 2013 at 12:32:44PM -0700, Robert Stone wrote: > On Tue, Oct 15, 2013 at 07:31:56AM +0100, William S Fulton wrote: > > Is there much more to do or is it ready now for merging? > > There is not much remaining. I am working on expanding the > Perl5 director docs, and that process has me looking at things pretty > closely. I'm not confident about how director destructors are currently > handled, but I should be able to clean it up in the next couple of days. This took longer than I'd hoped, but I think destructors are in far better shape now. Unfortunately, it comes at the cost of breaking an aspect of reverse compatibility. Some of SWIGs perl5 examples showed calling "$obj->DESTROY()" as the proper way to release objects. I haven't been able to make this use pattern play well with normal object cleanup now that directors can be involved. With more work, I might be able to make an explicit DESTROY call the only way to release a SWIG wrapped object, but it strikes me as the wrong thing to do. I am pretty sure the Perl community would find this just as awkward as if a C++ library suggested you run "obj->~Obj()" to release objects instead of "delete obj". I think the branch is ready for merge unless more work is needed to suppress that incompatibility. -Robert |
From: William S F. <ws...@fu...> - 2013-10-21 19:55:04
|
On 20/10/13 21:08, Robert Stone wrote: > On Tue, Oct 15, 2013 at 12:32:44PM -0700, Robert Stone wrote: >> On Tue, Oct 15, 2013 at 07:31:56AM +0100, William S Fulton wrote: >>> Is there much more to do or is it ready now for merging? >> >> There is not much remaining. I am working on expanding the >> Perl5 director docs, and that process has me looking at things pretty >> closely. I'm not confident about how director destructors are currently >> handled, but I should be able to clean it up in the next couple of days. > > This took longer than I'd hoped, but I think destructors are in > far better shape now. Unfortunately, it comes at the cost of breaking > an aspect of reverse compatibility. Some of SWIGs perl5 examples showed > calling "$obj->DESTROY()" as the proper way to release objects. I > haven't been able to make this use pattern play well with normal object > cleanup now that directors can be involved. > > With more work, I might be able to make an explicit DESTROY call > the only way to release a SWIG wrapped object, but it strikes me as the > wrong thing to do. I am pretty sure the Perl community would find this > just as awkward as if a C++ library suggested you run "obj->~Obj()" to > release objects instead of "delete obj". > The problem is if this was the shown to be a normal way to delete objects, then a lot of code is going to break. I can see the documentation also has examples like this. The approach taken might be a relic of before proxy objects were implemented. In fact does -noproxy still work? I don't think we have any tests for -noproxy. > I think the branch is ready for merge unless more work is needed > to suppress that incompatibility. Breaking the way objects are destroyed is far from a corner case and pretty fundamental, I feel we should add this back in. Sorry! Is there something special about DESTROY, can it not be added as an extra legacy wrapper method around destructors, perhaps via a feature? William |
From: Robert S. <ta...@tr...> - 2013-10-22 02:00:18
|
On Mon, Oct 21, 2013 at 08:54:48PM +0100, William S Fulton wrote: > On 20/10/13 21:08, Robert Stone wrote: > >On Tue, Oct 15, 2013 at 12:32:44PM -0700, Robert Stone wrote: > >>On Tue, Oct 15, 2013 at 07:31:56AM +0100, William S Fulton wrote: > >>>Is there much more to do or is it ready now for merging? > >> > >> There is not much remaining. I am working on expanding the > >>Perl5 director docs, and that process has me looking at things pretty > >>closely. I'm not confident about how director destructors are currently > >>handled, but I should be able to clean it up in the next couple of days. > > > > This took longer than I'd hoped, but I think destructors are in > >far better shape now. Unfortunately, it comes at the cost of breaking > >an aspect of reverse compatibility. Some of SWIGs perl5 examples showed > >calling "$obj->DESTROY()" as the proper way to release objects. I > >haven't been able to make this use pattern play well with normal object > >cleanup now that directors can be involved. > > > > With more work, I might be able to make an explicit DESTROY call > >the only way to release a SWIG wrapped object, but it strikes me as the > >wrong thing to do. I am pretty sure the Perl community would find this > >just as awkward as if a C++ library suggested you run "obj->~Obj()" to > >release objects instead of "delete obj". > > > The problem is if this was the shown to be a normal way to delete > objects, then a lot of code is going to break. I can see the > documentation also has examples like this. The approach taken might > be a relic of before proxy objects were implemented. In fact does > -noproxy still work? I don't think we have any tests for -noproxy. > > > I think the branch is ready for merge unless more work is needed > >to suppress that incompatibility. > > Breaking the way objects are destroyed is far from a corner case and > pretty fundamental, I feel we should add this back in. Sorry! Is > there something special about DESTROY, can it not be added as an > extra legacy wrapper method around destructors, perhaps via a > feature? I have been testing -noproxy mode, but nowhere near as heavily as the default mode. This is mainly because I don't know how to add test-suite entries that run swig with alternate command line options. Things work there, but the implicit argument to static member functions feels awkward in that mode since in that mode SWIG is not emitting a class for you, and doesn't do anything you can see with that argument either. I suspect I do have some additional reverse compatibility problems with some of the other command line options, particularly -compat and -nocppcast; I haven't been testing against those. I will try to work on this further. It probably makes sense to investigate not trying to unify the director proxy system with the existing one. If I can do that, retaining complete reverse compatibility should be trivial. -Robert |
From: William S F. <ws...@fu...> - 2013-10-22 18:21:17
|
On 22/10/13 03:00, Robert Stone wrote: > On Mon, Oct 21, 2013 at 08:54:48PM +0100, William S Fulton wrote: >> On 20/10/13 21:08, Robert Stone wrote: >>> On Tue, Oct 15, 2013 at 12:32:44PM -0700, Robert Stone wrote: >>>> On Tue, Oct 15, 2013 at 07:31:56AM +0100, William S Fulton wrote: >>>>> Is there much more to do or is it ready now for merging? >>>> >>>> There is not much remaining. I am working on expanding the >>>> Perl5 director docs, and that process has me looking at things pretty >>>> closely. I'm not confident about how director destructors are currently >>>> handled, but I should be able to clean it up in the next couple of days. >>> >>> This took longer than I'd hoped, but I think destructors are in >>> far better shape now. Unfortunately, it comes at the cost of breaking >>> an aspect of reverse compatibility. Some of SWIGs perl5 examples showed >>> calling "$obj->DESTROY()" as the proper way to release objects. I >>> haven't been able to make this use pattern play well with normal object >>> cleanup now that directors can be involved. >>> >>> With more work, I might be able to make an explicit DESTROY call >>> the only way to release a SWIG wrapped object, but it strikes me as the >>> wrong thing to do. I am pretty sure the Perl community would find this >>> just as awkward as if a C++ library suggested you run "obj->~Obj()" to >>> release objects instead of "delete obj". >>> >> The problem is if this was the shown to be a normal way to delete >> objects, then a lot of code is going to break. I can see the >> documentation also has examples like this. The approach taken might >> be a relic of before proxy objects were implemented. In fact does >> -noproxy still work? I don't think we have any tests for -noproxy. >> >>> I think the branch is ready for merge unless more work is needed >>> to suppress that incompatibility. >> >> Breaking the way objects are destroyed is far from a corner case and >> pretty fundamental, I feel we should add this back in. Sorry! Is >> there something special about DESTROY, can it not be added as an >> extra legacy wrapper method around destructors, perhaps via a >> feature? > > I have been testing -noproxy mode, but nowhere near as heavily > as the default mode. This is mainly because I don't know how to add > test-suite entries that run swig with alternate command line options. > Things work there, but the implicit argument to static member functions > feels awkward in that mode since in that mode SWIG is not emitting a > class for you, and doesn't do anything you can see with that argument > either. I suspect I do have some additional reverse compatibility > problems with some of the other command line options, particularly > -compat and -nocppcast; I haven't been testing against those. > It is possible to add in extra commandline options per testcase. Examples/test-suite/csharp/Makefile has such an example: # Custom tests - tests with additional commandline options intermediary_classname.cpptest: SWIGOPT += -dllimport intermediary_classname and for all languages, see common.mk, eg: # Custom tests - tests with additional commandline options wallkw.cpptest: SWIGOPT += -Wallkw preproc_include.ctest: SWIGOPT += -includeall By all means add in one or two non-standard commandline tests, but really most of the test-suite should run using the default options. > I will try to work on this further. It probably makes sense to > investigate not trying to unify the director proxy system with the > existing one. If I can do that, retaining complete reverse > compatibility should be trivial. What do you mean by this? There is no director support in Perl on master. Perhaps you don't want to use all the common director support code? If so, I'd really want to understand why and look at ways to keep the code common. William |
From: Robert S. <ta...@tr...> - 2013-10-22 21:46:09
|
On Tue, Oct 22, 2013 at 07:21:02PM +0100, William S Fulton wrote: > It is possible to add in extra commandline options per testcase. > Examples/test-suite/csharp/Makefile has such an example: > > # Custom tests - tests with additional commandline options > intermediary_classname.cpptest: SWIGOPT += -dllimport intermediary_classname > > and for all languages, see common.mk, eg: > # Custom tests - tests with additional commandline options > wallkw.cpptest: SWIGOPT += -Wallkw > preproc_include.ctest: SWIGOPT += -includeall > > By all means add in one or two non-standard commandline tests, but > really most of the test-suite should run using the default options. That's great, thank you for the hints. > > I will try to work on this further. It probably makes sense to > >investigate not trying to unify the director proxy system with the > >existing one. If I can do that, retaining complete reverse > >compatibility should be trivial. > What do you mean by this? There is no director support in Perl on > master. Perhaps you don't want to use all the common director > support code? If so, I'd really want to understand why and look at > ways to keep the code common. I mean that I think I can significantly reduce the risk of this feature by re-cutting my work as a set of enhancements that only affect wrappers when directors are enabled. I have been trying to deeply change the way SWIG handles Perl, but I think this might be a mistake. I have not been thinking about "-noproxy" mode much and I expect to find other compatibility problems. Mechanically, this would be trying to change the proposal to a set of additions to the code wrapped in "if (directorsEnabled()) {}" conditions as much as possible. That would to make the static member function compatibility issue vanish, and solve the legacy destructor problem. It should also mean the code can continue to support perl v5.6 when directors are not needed. One downside would be that SWIG_GetModule() wouldn't be able to share objects between director enabled and directorless modules. And, of course, enabling directors for existing code would trigger the altered static member function and destructor semantics. -Robert |