edgedb-python icon indicating copy to clipboard operation
edgedb-python copied to clipboard
verified publisher icon edgedb

Redo edgedb basic types to inherit from builtin types

Open fantix opened this issue 3 years ago • 5 comments

  • [x] edgedb.Tuple is tuple
    • weakref.ref() will stop working on edgedb.Tuple objects.
  • [x] edgedb.NamedTuple is a subclass of tuple
    • Actual named tuple objects will be an instance of a transient heap type DerivedNamedTuple (subclass of edgedb.NamedTuple)
    • weakref.ref() will stop working on edgedb.NamedTuple objects.
  • [ ] edgedb.Set is a subclass of tuple
  • [ ] edgedb.Array is a subclass of tuple
  • [ ] link and linkprop?

fantix avatar Sep 14 '22 18:09 fantix

https://github.com/edgedb/edgedb-python/blob/dfb8c8b009838eec44a312d467edd4782197897f/edgedb/datatypes/array.c#L47

Will other types from stdlib such as arrays inherit from builtin?

i0bs avatar Sep 14 '22 18:09 i0bs

Will other types from stdlib such as arrays inherit from builtin?

Yes, but array may just be a subclass of tuple because it's immutable.

fantix avatar Sep 14 '22 18:09 fantix

Yes, but array may just be a subclass of tuple because it's immutable.

Has it always been immutable? I thought EdgeQL supported mutation to the shape and size of arrays.

i0bs avatar Sep 14 '22 18:09 i0bs

Has it always been immutable?

Oh this PR is the edgedb-python construct of arrays only in query results, it was and will be immutable.

fantix avatar Sep 14 '22 19:09 fantix

Why can't we use the builtin tuple type for our tuples? The __repr__ is basically compatible, so maybe just drop our custom tuple implementation and use the builtin one? It has a freelist optimization.

1st1 avatar Sep 15 '22 23:09 1st1

Has it always been immutable?

Oh this PR is the edgedb-python construct of arrays only in query results, it was and will be immutable.

UPDATE: we decided to use plain Python list for edgedb.Set and edgedb.Array, so the query results will be partially mutable, but again that's just in user memory.

fantix avatar Sep 23 '22 22:09 fantix

UPDATE: we decided to use plain Python list for edgedb.Set and edgedb.Array, so the query results will be partially mutable, but again that's just in user memory.

Does the builtin list object support the same optimisation as before? Does this improve querying perf?

i0bs avatar Sep 26 '22 14:09 i0bs

UPDATE: we decided to use plain Python list for edgedb.Set and edgedb.Array, so the query results will be partially mutable, but again that's just in user memory.

Does the builtin list object support the same optimisation as before? Does this improve querying perf?

The main optimization is still here, which is using the C API PyList_SET_ITEM for fast initialization, but we lost the (cached) hashing function, which is not a critical use. Also we lost the freelist of array, which may have slight performance impact when arrays are frequently (re-)allocated. Other than that, query performance should not be affected, I’ll run benchmarks to test it.

fantix avatar Sep 26 '22 15:09 fantix

I'd love to hear more about the benchmarks once they're available. This sounds like a great change if memory allocation doesn't prove to be too impactful for builtin inheritance. Great stuff @fantix ❤️

i0bs avatar Sep 26 '22 16:09 i0bs

@i0bs benchmark updated

fantix avatar Sep 28 '22 01:09 fantix