You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucy.apache.org by "Nick Wellnhofer (JIRA)" <ji...@apache.org> on 2012/05/01 20:58:49 UTC
[lucy-issues] [jira] [Created] (LUCY-233) Implement Clownfish Method class
Nick Wellnhofer created LUCY-233:
------------------------------------
Summary: Implement Clownfish Method class
Key: LUCY-233
URL: https://issues.apache.org/jira/browse/LUCY-233
Project: Lucy
Issue Type: Task
Components: Clownfish
Reporter: Nick Wellnhofer
As discussed on lucy-dev, it would be nice to have a Clownfish class for methods. The VTables would then contain a VArray of Methods which can be used to setup host language overrides and later to lookup offsets dynamically.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[lucy-issues] [jira] [Updated] (LUCY-233) Implement Clownfish Method class
Posted by "Marvin Humphrey (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCY-233?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Marvin Humphrey updated LUCY-233:
---------------------------------
Attachment: take1_take2.diff
The attached file "take1_take2.diff" shows the difference between
the first four patches of patchset 1 and patchset 2.
> Implement Clownfish Method class
> --------------------------------
>
> Key: LUCY-233
> URL: https://issues.apache.org/jira/browse/LUCY-233
> Project: Lucy
> Issue Type: Task
> Components: Clownfish
> Reporter: Nick Wellnhofer
> Attachments: 0001-Rename-METHOD-macro-to-METHOD_PTR.patch, 0002-Rework-VTable-bootstrapping.patch, 0003-Initial-implementation-of-Method-class.patch, 0004-Rework-VTable-method-initialization.patch, 0101-Rename-METHOD-macro-to-METHOD_PTR.patch, 0102-Rework-VTable-bootstrapping.patch, 0103-Initial-implementation-of-Method-class.patch, 0104-Rework-VTable-method-initialization.patch, 0105-Autogenerate-an-array-with-initialization-data-for-a.patch, take1_take2.diff
>
>
> As discussed on lucy-dev, it would be nice to have a Clownfish class for methods. The VTables would then contain a VArray of Methods which can be used to setup host language overrides and later to lookup offsets dynamically.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[lucy-issues] [jira] [Commented] (LUCY-233) Implement Clownfish Method class
Posted by "Marvin Humphrey (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCY-233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13409959#comment-13409959 ]
Marvin Humphrey commented on LUCY-233:
--------------------------------------
Nice improvements! I like how lucy_bootstrap_parcel() has become shorter and
easier to grok now that it has a loop instead of a couple hundred function
calls. :) And thanks for addressing the handful of concerns from the initial
go-round.
+1 to commit the second patch sequence.
> Now that VTables are allocated on the heap, I'm not sure if overriding
> Inc_RefCount and Dec_RefCount is still needed. I think we could and should
> use a normal destructor now. The same for Methods.
Hmm, that would make these objects mutable -- and that poses a problem, as
they are shared globals. Clownfish is designed to support strongly divorced
concurrency in threads; VTables -- and now Methods -- are shared, but they are
the *only* objects which can be shared. Having VTables be shared singletons
is a deeply-baked-in assumption for the current design. If we make them
mutable, we will still need to override all refcount manipulation and access
methods, to provide for threadsafe incrementing and decrementing -- the
default refcount manipulation methods inherited from Obj are not atomic.
We would also have problems if VTables or Methods were to be freed before any
objects which hold references to them. This can happen under out-of-order
refcount decrementing during Perl's global destruction phase:
http://markmail.org/message/52i2n7f2ma5jlajc
> Implement Clownfish Method class
> --------------------------------
>
> Key: LUCY-233
> URL: https://issues.apache.org/jira/browse/LUCY-233
> Project: Lucy
> Issue Type: Task
> Components: Clownfish
> Reporter: Nick Wellnhofer
> Attachments: 0001-Rename-METHOD-macro-to-METHOD_PTR.patch, 0002-Rework-VTable-bootstrapping.patch, 0003-Initial-implementation-of-Method-class.patch, 0004-Rework-VTable-method-initialization.patch, 0101-Rename-METHOD-macro-to-METHOD_PTR.patch, 0102-Rework-VTable-bootstrapping.patch, 0103-Initial-implementation-of-Method-class.patch, 0104-Rework-VTable-method-initialization.patch, 0105-Autogenerate-an-array-with-initialization-data-for-a.patch, take1_take2.diff
>
>
> As discussed on lucy-dev, it would be nice to have a Clownfish class for methods. The VTables would then contain a VArray of Methods which can be used to setup host language overrides and later to lookup offsets dynamically.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[lucy-issues] [jira] [Commented] (LUCY-233) Implement Clownfish Method class
Posted by "Nick Wellnhofer (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCY-233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13414402#comment-13414402 ]
Nick Wellnhofer commented on LUCY-233:
--------------------------------------
OK, now I understand why we need the special refcount handling.
> Implement Clownfish Method class
> --------------------------------
>
> Key: LUCY-233
> URL: https://issues.apache.org/jira/browse/LUCY-233
> Project: Lucy
> Issue Type: Task
> Components: Clownfish
> Reporter: Nick Wellnhofer
> Attachments: 0001-Rename-METHOD-macro-to-METHOD_PTR.patch, 0002-Rework-VTable-bootstrapping.patch, 0003-Initial-implementation-of-Method-class.patch, 0004-Rework-VTable-method-initialization.patch, 0101-Rename-METHOD-macro-to-METHOD_PTR.patch, 0102-Rework-VTable-bootstrapping.patch, 0103-Initial-implementation-of-Method-class.patch, 0104-Rework-VTable-method-initialization.patch, 0105-Autogenerate-an-array-with-initialization-data-for-a.patch, take1_take2.diff
>
>
> As discussed on lucy-dev, it would be nice to have a Clownfish class for methods. The VTables would then contain a VArray of Methods which can be used to setup host language overrides and later to lookup offsets dynamically.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[lucy-issues] [jira] [Resolved] (LUCY-233) Implement Clownfish Method class
Posted by "Nick Wellnhofer (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCY-233?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Nick Wellnhofer resolved LUCY-233.
----------------------------------
Resolution: Fixed
Assignee: Nick Wellnhofer
Committed the second patch series and an additional commit that disables refcounting of Methods.
> Implement Clownfish Method class
> --------------------------------
>
> Key: LUCY-233
> URL: https://issues.apache.org/jira/browse/LUCY-233
> Project: Lucy
> Issue Type: Task
> Components: Clownfish
> Reporter: Nick Wellnhofer
> Assignee: Nick Wellnhofer
> Attachments: 0001-Rename-METHOD-macro-to-METHOD_PTR.patch, 0002-Rework-VTable-bootstrapping.patch, 0003-Initial-implementation-of-Method-class.patch, 0004-Rework-VTable-method-initialization.patch, 0101-Rename-METHOD-macro-to-METHOD_PTR.patch, 0102-Rework-VTable-bootstrapping.patch, 0103-Initial-implementation-of-Method-class.patch, 0104-Rework-VTable-method-initialization.patch, 0105-Autogenerate-an-array-with-initialization-data-for-a.patch, take1_take2.diff
>
>
> As discussed on lucy-dev, it would be nice to have a Clownfish class for methods. The VTables would then contain a VArray of Methods which can be used to setup host language overrides and later to lookup offsets dynamically.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[lucy-issues] [jira] [Commented] (LUCY-233) Implement Clownfish Method class
Posted by "Marvin Humphrey (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCY-233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13266838#comment-13266838 ]
Marvin Humphrey commented on LUCY-233:
--------------------------------------
Looks good, +1 to commit!
Comments:
* The METHOD_PTR patch looks fine.
* I chuckled at the "Bootstrap VTables" hacks necessary when the the parcel
prefix is "lucy_" (which will eventually change to "cfish_") so that
VTable_init() could be generalized out of VTable_bootstrap().
* Nice adding comments in the generated code. That makes both the transpiler
output and the sprintf format string easier to grok.
* FWIW, the variable "x" which is "reserved for future use" in VTable can go
away. It's an artifact of when OFFSET vars had fixed values.
* I think it makes sense for Method objects to become immutable, like
VTables (modulo the mutable host object bug for VTables). That would mean
overriding Inc_RefCount() and Dec_RefCount() and leaking them
intentionally.
* It's a long-term concern that VTables now contain more elements that aren't
immutable, e.g. VArrays. Our goals for thread support are minimal -- we
only support strongly divorced concurrency -- but if VTables and their
components are not immutable, we'll have thread safety issues. Perhaps we
need classes like "FinalString", "FinalHash", etc. to support situations
like this.
* I notice that the Method takes ownership of the CharBuf "name" in
Method_init() in the third patch. What we do elsewhere is pass in a "const
CharBuf*" and CB_Clone() it, so that the caller doesn't have to worry about
whether they modify it. The same thing applies to VTable_init() in patch
4.
* CB_newf(name) might be better written as CB_newf("%s", name), since the
first arg is a format string.
> Implement Clownfish Method class
> --------------------------------
>
> Key: LUCY-233
> URL: https://issues.apache.org/jira/browse/LUCY-233
> Project: Lucy
> Issue Type: Task
> Components: Clownfish
> Reporter: Nick Wellnhofer
> Attachments: 0001-Rename-METHOD-macro-to-METHOD_PTR.patch, 0002-Rework-VTable-bootstrapping.patch, 0003-Initial-implementation-of-Method-class.patch, 0004-Rework-VTable-method-initialization.patch
>
>
> As discussed on lucy-dev, it would be nice to have a Clownfish class for methods. The VTables would then contain a VArray of Methods which can be used to setup host language overrides and later to lookup offsets dynamically.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[lucy-issues] [jira] [Updated] (LUCY-233) Implement Clownfish Method class
Posted by "Nick Wellnhofer (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCY-233?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Nick Wellnhofer updated LUCY-233:
---------------------------------
Attachment: 0105-Autogenerate-an-array-with-initialization-data-for-a.patch
0104-Rework-VTable-method-initialization.patch
0103-Initial-implementation-of-Method-class.patch
0102-Rework-VTable-bootstrapping.patch
0101-Rename-METHOD-macro-to-METHOD_PTR.patch
Here is a reworked patchset (0101-0105) addressing some of your comments. Patch 0105 reworks the VTable initialization even more. I think it's best to first look at the generated code in autogen/sources/parcel.c to understand the changes. IMO it makes the whole initialization process more flexible.
Now that VTables are allocated on the heap, I'm not sure if overriding Inc_RefCount and Dec_RefCount is still needed. I think we could and should use a normal destructor now. The same for Methods.
> Implement Clownfish Method class
> --------------------------------
>
> Key: LUCY-233
> URL: https://issues.apache.org/jira/browse/LUCY-233
> Project: Lucy
> Issue Type: Task
> Components: Clownfish
> Reporter: Nick Wellnhofer
> Attachments: 0001-Rename-METHOD-macro-to-METHOD_PTR.patch, 0002-Rework-VTable-bootstrapping.patch, 0003-Initial-implementation-of-Method-class.patch, 0004-Rework-VTable-method-initialization.patch, 0101-Rename-METHOD-macro-to-METHOD_PTR.patch, 0102-Rework-VTable-bootstrapping.patch, 0103-Initial-implementation-of-Method-class.patch, 0104-Rework-VTable-method-initialization.patch, 0105-Autogenerate-an-array-with-initialization-data-for-a.patch
>
>
> As discussed on lucy-dev, it would be nice to have a Clownfish class for methods. The VTables would then contain a VArray of Methods which can be used to setup host language overrides and later to lookup offsets dynamically.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[lucy-issues] [jira] [Updated] (LUCY-233) Implement Clownfish Method class
Posted by "Nick Wellnhofer (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCY-233?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Nick Wellnhofer updated LUCY-233:
---------------------------------
Attachment: 0004-Rework-VTable-method-initialization.patch
0003-Initial-implementation-of-Method-class.patch
0002-Rework-VTable-bootstrapping.patch
0001-Rename-METHOD-macro-to-METHOD_PTR.patch
Patches 01-04 contain an initial implementation of the Method class and a rework of the autogenerated VTable initialization code.
> Implement Clownfish Method class
> --------------------------------
>
> Key: LUCY-233
> URL: https://issues.apache.org/jira/browse/LUCY-233
> Project: Lucy
> Issue Type: Task
> Components: Clownfish
> Reporter: Nick Wellnhofer
> Attachments: 0001-Rename-METHOD-macro-to-METHOD_PTR.patch, 0002-Rework-VTable-bootstrapping.patch, 0003-Initial-implementation-of-Method-class.patch, 0004-Rework-VTable-method-initialization.patch
>
>
> As discussed on lucy-dev, it would be nice to have a Clownfish class for methods. The VTables would then contain a VArray of Methods which can be used to setup host language overrides and later to lookup offsets dynamically.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira