Guys, I was offline a few days; happy to see the way this issue is resolved (both APIs retained).
Re Weldon's vmmagic contract concerns: the field_is_gc_enumerable() was introduced to address this in the first place. -- Alexey 2007/3/8, Xiao-Feng Li <[EMAIL PROTECTED]>:
Thanks to Rana and quick hand! xiaofeng On 3/8/07, Rana Dasgupta <[EMAIL PROTECTED]> wrote: > Assuming that we are OK with this, I added the discussed changes in > > https://issues.apache.org/jira/browse/HARMONY-3330 > > Thanks, > Rana > > > On 3/7/07, Rana Dasgupta <[EMAIL PROTECTED]> wrote: > > > > Looks like the jvmti heap tracing/profiling functionality also calls this. > > It may be better to add it to the general interface just implement it in the > > VM to avoid surpirises ( it also avoids duplicating in all the gc's ). > > Sounds OK? > > > > Thanks, > > Rana > > > > > > On 3/6/07, Xiao-Feng Li <[EMAIL PROTECTED]> wrote: > > > > > > Alexey, I think it's probably good idea to add field_is_reference() > > > and field_is_magic(), to replace the field_is_gc_enumerable() > > > interface, since it gives the user of the interfaces more flexibility > > > and probably future-proof. Then we have: > > > > > > bool field_is_enumerable_reference() > > > { > > > return field_is_reference() && !field_is_magic(); > > > } > > > > > > This function can be implemented by GC itself if it wants to abstract > > > it. As you mentioned, if its only customer is GC module, there is no > > > need to put it in the general interface. > > > > > > There is no need to implement field_is_primitive() because it's > > > !field_is_reference(). > > > > > > How do you think? > > > > > > Thanks, > > > xiaofeng > > > > > > On 3/7/07, Alexey Varlamov <[EMAIL PROTECTED]> wrote: > > > > Apparently field_is_gc_enumerable() has it's customers, and reasonably > > > > > > > shields them from unrelated low-level details - be it magics or some > > > > other vm-injected data. I'm OK to rename it to > > > > field_is_enumerable_reference() or any other name which is considered > > > > more descriptive. > > > > And what purpose do you see for the plain accessor > > > > field_is_reference()? Do we need field_is_primitive() and/or > > > > field_is_magic() also? Note there is already field_get_descriptor() > > > > which basically provides this kind of info. If we have no clear > > > > usecase, I'd prefer to keep interface smaller. > > > > > > > > -- > > > > Alexey > > > > > > > > 2007/3/6, Rana Dasgupta <[EMAIL PROTECTED]>: > > > > > I don't have a strong opinion. But is_reference is a field sematic, > > > it may > > > > > not matter who consumes it at the moment. Also, not implementing an > > > > > interface is OK, but not because noone needs it, it is a constraint > > > of the > > > > > implementer. That's valid, but in that case, one needs to go away > > > from > > > > > Boolean and support a NOT_IMPLEMENT return value also. Sorry if I am > > > > > sounding convoluted. > > > > > > > > > > Rana > > > > > > > > > > > > > > > On 3/6/07, Mikhail Fursov < [EMAIL PROTECTED]> wrote: > > > > > > > > > > > > field_is_reference() was used only in GC and was not used by > > > other code. > > > > > > This is the reason why original 'field_is_reference' was not kept. > > > > > > > > > We can rename 'field_is_gc_enumerable' to > > > 'field_is_enumerable_reference' > > > > > > and do not implement field_is_reference() method (unless someone > > > needs > > > > > > it). > > > > > > Does it makes sense? As for me both names are good. > > > > > > > > > > > > field_is_magic_addr() does not look good to me. It has too many > > > details > > > > > > about magics in its name, while the only knowledge we need today > > > is to > > > > > > know > > > > > > if to enumerate a field or not. > > > > > > > > > > > > > > > > > > On 3/6/07, Rana Dasgupta <[EMAIL PROTECTED]> wrote: > > > > > > > > > > > > > > Good point. In fact, field attributes seem better connected to > > > field > > > > > > > semantics, not to GC requirements directly. Is it possible to > > > retain > > > > > > > field_is_reference() and add a field_is_magic_addr() ? Though > > > there is > > > > > > an > > > > > > > implied inefficiency here, the semantics seem clearer.Are there > > > other > > > > > > > magic > > > > > > > field types that could interfere? > > > > > > > > > > > > > > Thanks, > > > > > > > Rana > > > > > > > > > > > > > > > > > > > > > On 3/6/07, Xiao-Feng Li <[EMAIL PROTECTED] > wrote: > > > > > > > > > > > > > > > > Hi, I found field_is_reference in original vm.h was changed to > > > be > > > > > > > > field_is_gc_enumerable. The declaration is: > > > > > > > > > > > > > > > > /** > > > > > > > > * @return <code>TRUE</code> if the field must be enumerated by > > > GC > > > > > > > > * > > > > > > > > * This function doesn't cause resolution of the class of the > > > field. > > > > > > > > */ > > > > > > > > VMEXPORT Boolean field_is_gc_enumerable(Field_Handle fh); > > > > > > > > > > > > > > > > > > > > > > > > I wonder what is the rationality to make this interface > > > change. > > > > > > > > > > > > > > > > From reading the code, I guess this change was made due to the > > > > > > > > implementation Magics. With Magics, a reference field may not > > > always > > > > > > > > be enumerated by the VM during garbage collection, such as > > > Address > > > > > > > > field in a Java helper. To change the function name to be > > > > > > > > "field_is_gc_enumerable" might help the reader to know this > > > fact. > > > > > > > > > > > > > > > > But I think this doesn't actually help, since the user of this > > > > > > > > function will be confused about the type of the field, and > > > need to > > > > > > > > guess what kind of field is "gc enumerable". More importantly, > > > the > > > > > > > > semantics of this function are unclear: it hard-encodes the > > > > > > > > Magics-related semantics into the low-level field accessors. > > > > > > > > > > > > > > > > I would suggest to keep the original field_is_reference > > > interface > > > > > > > > function in this vm.h file. It clearly tells if a field is > > > reference > > > > > > > > type. If we really want the field_is_gc_enumerable interface, > > > we can > > > > > > > > add it as a new one. We can use a new name like > > > > > > > > "field_is_enumerable_reference", which is probably clearer. > > > > > > > > > > > > > > > > How about it? > > > > > > > > > > > > > > > > Thanks, > > > > > > > > xiaofeng > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > Mikhail Fursov > > > > > > > > > > > > > > > > > > > > > > >
