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

add interval tree #63

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from
Draft

add interval tree #63

wants to merge 12 commits into from

Conversation

dwuggh
Copy link

@dwuggh dwuggh commented Mar 9, 2024

This is an interval tree implementation based on red-black tree. Merging intervals still need work, as well as updating plists.

My goal is to validate ideas in #61 , an elisp-managed concurrent (re)display models for emacs.

Emacs' implementation

emacs store every buffer or strings text property in an interval tree, defined in intervals.h:

struct interval
{
  /* The first group of entries deal with the tree structure.  */
  ptrdiff_t total_length;       /* Length of myself and both children.  */
  ptrdiff_t position;	        /* Cache of interval's character position.  */
                                /* This field is valid in the final
                                   target interval returned by
                                   find_interval, next_interval,
                                   previous_interval and
                                   update_interval.  It cannot be
                                   depended upon in any intermediate
                                   intervals traversed by these
                                   functions, or any other
                                   interval. */
  struct interval *left;	/* Intervals which precede me.  */
  struct interval *right;	/* Intervals which succeed me.  */

  /* Parent in the tree, or the Lisp_Object containing this interval tree.  */
  union
  {
    struct interval *interval;
    Lisp_Object obj;
  } up;
  bool_bf up_obj : 1;

  bool_bf gcmarkbit : 1;

  /* The remaining components are `properties' of the interval.
     The first four are duplicates for things which can be on the list,
     for purposes of speed.  */

  bool_bf write_protect : 1;	    /* True means can't modify.  */
  bool_bf visible : 1;		    /* False means don't display.  */
  bool_bf front_sticky : 1;	    /* True means text inserted just
				       before this interval goes into it.  */
  bool_bf rear_sticky : 1;	    /* Likewise for just after it.  */
  Lisp_Object plist;		    /* Other properties.  */
};
  • text properties are defined as plists, as in the plist field. Relevant functions are in textprop.c and intervals.h
  • when redisplay, the iterator for constructing glyph matrix it finds next interval of text props for redisplay in compute_stop_pos, in xdisp.c.
  • These intervals never intersect; they are splitted and merged during insertion, i.e. set-text-properties.
  • the position field, is updated during redisplay, to optimize for performance.

This implementation

The interval tree is based on a red-black tree, algorithms adapted from the famous book Dr. Sergewick's /Algorithms/, 4th ed.

I've looked at rust-bio's implementation as you mentioned in design.org, the arraybacked one seems not so efficient on deletion, and the AVL tree impl is similar to this one.

@dwuggh dwuggh marked this pull request as draft March 9, 2024 11:19
@CeleritasCelery
Copy link
Owner

Thanks for this! Is this code based on another crate or an existing implementation? Do you see these intervals being used for text properties or overlays? I am not sure what the association is with plists, because I believe that plists are just regular lists are the hood.

@dwuggh
Copy link
Author

dwuggh commented Mar 10, 2024

just updated add_properties in textprops.rs, in accrodance to textprop.c, and some contents in the first post.

This is based on an textbook's rbtree implementation(/Algorithms/, 4th ed). I'm not familiar with overlays, but text properties are stored as plists, like (:face some-face :display some-display-prop). They indeed are normal list with keywords; I wrote the add_properties API to deal with them, but got stuck at creating new lists.

src/textprops.rs Outdated
Comment on lines 60 to 65
// if no key1 found, append them
if !found {
let pl = plist_i.untag();
changed = true;
let new_cons: Gc<ListType<'ob>> = Cons::new(key1, Cons::new(val1?, pl, cx), cx).into();
plist_i = new_cons; // TODO this does not work
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How should I turn a Nil Cons to a contented one?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

anything that is a subset of an Object can be converted to an Object via into. So if you wanted to update plist_i you can first make it mutable

pub fn add_properties<'ob>(
...
mut plist_i: Object<'ob>,

then update it:

let new_cons: List = Cons::new(key1, Cons::new(val1?, pl, cx), cx).into();
 plist_i = new_cons.into();

or even just like this

plist_i = Cons::new(key1, Cons::new(val1?, pl, cx), cx).into();

Copy link
Author

@dwuggh dwuggh Mar 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for this, actually it is just how I did. now i'm passing references plist_i: &mut Object<'ob> to modify the plist inplace. i wonder whether this style is preferred.

Copy link
Owner

@CeleritasCelery CeleritasCelery Mar 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that Object is just a tagged pointer, so you can't modify the underlying data via &mut (it would be similar to having &mut &String, where you have a mutable reference to an immutable string). if plist_i is a Cons, than you can use .set_car() or .set_cdr().

Though looking at what you are trying to do, we want to preprend key1. But that will change the head of the list, so we can't do it mutably. Instead we should return a new plist_i and then reassign it outside of add_properties. Either that or append key1 (instead of preprend) and that will modify the existing list without changing the head.

Technically you are doing the same thing with &mut Object (changing the head), but since Object is copy, I think it is more idiomatic to return the updated value.

@CeleritasCelery
Copy link
Owner

I added a arch doc that will hopefully help make it clearer how the types interact and how to do basic things. If you feel something is missing, please let me know.

@dwuggh
Copy link
Author

dwuggh commented Mar 28, 2024

got another question about Env and rooting:
I'm adding the interval tree to Env:

#[derive(Debug, Default, Trace)]
pub struct Env<'a> {
    //...
    pub buffer_textprops: ObjectMap<String, Slot<IntervalTree<'a>>>,
}

but trace cannot be derived for IntervalTree.

The IntervalTree has following structure:

struct IntervalTree<'ob> {
    root: Node<'ob>,
}

struct Node<'ob> {
    //...
    val: Object<'ob>, // text property plist
}

This object should be rooted; but how should I supposed to do so?

@CeleritasCelery
Copy link
Owner

CeleritasCelery commented Jun 6, 2024

got another question about Env and rooting:

@dwuggh Sorry I somehow missed this comment.

Slot is only for types that will be moved by the GC (i.e. Objects), so we don't need it here.

What we need is to put Slot in Node.

struct Node<'ob> {
    //...
    val: Slot<Object<'ob>>, // text property plist
}

Then we should be able to derive Trace for Node and IntervalTree.

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