Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

This patch makes defstructs able to be redefined. #498

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alanruttenberg
Copy link
Collaborator

Currently, ABCL catches the case where the defstruct has been changed and throws an
error. The main issue with defstruct is that when compiling, the
accessors are inlined as position-based accessors. So

(defstruct a b)

will compile a-b into lispObject.getSlotValue_0().

If another slot is added at the beginning
(defstruct a first b)

Then using a-b will get the value of the slot first instead of b.

The mechanism that ABCL uses to do inline is a source
transform. Allowing redefinition prevents that source transformation
from being created. Without the source transform, the compiled accessors
get the correct slot, at the cost of some performance.

Redefinition is controlled by a global variable
allow-defstruct-redefinition and a new option to defstruct :optimize.
If allow-defstruct-redefinition is t then

(defstruct (a :optimize nil) b)

Will prevent the source transform from being created. Otherwise the
current ABCL behavior is replicated.

The way ABCL detects that a change has been made is by saving the slot
definitions in the structure class, in a map of class names to
class-object defined in LispClass. The class object has a copy of the
slot definitions. One change is that we make is to make that map public.
There's an accessor findClass, but the result can't be cast to a
StructureClass, which the it needs to be in order to do the redefinition.

Then we add a new java function in StructureClass.java
reinitialize-structure-class, which replicates most of the logic of
make-structure-class, except that it reuses the old structure class
object. It's necessary to use the old value so that generic methods
specialized on the class will still work.

In defstruct.lisp there are changes to recognize the new option, to
check it before defining source transforms, and to call reinitialize-structure-class
instead of make-structure-class if the defstruct is already defined.

In the case that a defstruct is first created without the :optimize
option then subseqently modified to use :optimize nil, a warning is
given that previously compiled functions that use the accessors may need
to be recompiled and the previously defined source transforms are removed.
However, as long as the order of previously defined slots in the newly
defined defstruct remains the same as the order in the old structure,
inlined accessors will continue to work and recompilation is not
necessary.

The intended use of redefinition is during development, where structures
may evolve. There's no reason to use this mechanism in production code.

catches the case where the defstruct has been changed and throws an
error. The main issue with defstruct is that when compiling, the
accessors are inlined as position-based accessors. So

(defstruct a b)

will compile a-b into lispObject.getSlotValue_0().

If another slot is added at the beginning
(defstruct a first b)

Then using a-b will get the value of the slot first instead of b.

The mechanism that ABCL uses to do inline is a source
transform. Allowing redefinition prevents that source transformation
from being created. Without the source transform, the compiled accessors
get the correct slot, at the cost of some performance.

Redefinition is controlled by a global variable
*allow-defstruct-redefinition* and a new option to defstruct :optimize.
If *allow-defstruct-redefinition* is t then

(defstruct (a :optimize nil) b)

Will prevent the source transform from being created. Otherwise the
current ABCL behavior is replicated.

The way ABCL detects that a change has been made is by saving the slot
definitions in the structure class, in a map of class names to
class-object defined in LispClass. The class object has a copy of the
slot definitions. One change is that we make is to make that map public.
There's an accessor findClass, but the result can't be cast to a
StructureClass, which the it needs to be in order to do the redefinition.

Then we add a new java function in StructureClass.java
reinitialize-structure-class, which replicates most of the logic of
make-structure-class, except that it reuses the old structure class
object. It's necessary to use the old value so that generic methods
specialized on the class will still work.

In defstruct.lisp there are changes to recognize the new option, to
check it before defining source transforms, and to call reinitialize-structure-class
instead of make-structure-class if the defstruct is already defined.

In the case that a defstruct is first created without the :optimize
option then subseqently modified to use :optimize nil, a warning is
given that previously compiled functions that use the accessors may need
to be recompiled and the previously defined source transforms are removed.
However, as long as the order of previously defined slots in the newly
defined defstruct remains the same as the order in the old structure,
inlined accessors will continue to work and recompilation is not
necessary.

The intended use of redefinition is during development, where structures
may evolve. There's no reason to use this mechanism in production code.
@easye
Copy link
Collaborator

easye commented Nov 28, 2022

Potentially very useful, but since redefining structures is not allowed in ANSI, I think we should

  1. Add :structure-redefinition-possible to *features*

  2. Make this the non-default somehow

My reasoning behind not making this the default, as with the newer interfaces to memory, Lisp structs may be in memory outside of the Java heap, perhaps even memory mapped to the file system for which redefinition may be tricky.

Sorry for the late feedback.

@alanruttenberg
Copy link
Collaborator Author

Just wanted to emphasize that this is really only relevant for development. The situation is that you are iterating
and you need an extra slot or decide you don't need one. The expectation is that developers won't, if they use this, have existing structures that they should expect to work with the new definition, unless the change in definition adds a new slot and but there is no attempt to access that slot previously allocated structures. i.e. only previously defined slots on those structures can be accessed. But the more likely scenario is that structures allocated before the change are no longer in use and probably garbage collected.

Also, since use of this is only when using a non-standard option to the defstruct definition, I don't think we'll see accidental use. It might even be hidden behind a global switch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants