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.

Sign up for GitHub

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

Jump to bottom

[WIP] Extract vec without specialization #1

Closed
wants to merge 5 commits into from
Closed

[WIP] Extract vec without specialization #1

wants to merge 5 commits into from

Conversation

davidhewitt
Copy link
Owner

Attempt to implement FromPyObject for Vec<T> with a buffer optimisation, without specialisation.

Can't merge because:

In this new design, FromPyObject for Vec<String> is not currently implemented, but FromPyObject for String is. In general this is a problem for all types which implement FromPyObject instead of FromPyObjectImpl.

Probably the solution is to make it ergonomic to implement FromPyObjectImpl directly for all types.

@davidhewitt davidhewitt mentioned this pull request Jan 14, 2020
Merged
@davidhewitt
Copy link
Owner Author

I've come round to a design which is looking a lot cleaner. However, it still doesn't work for all types. The next step is probably to force implementations of FromPyObjectImpl rather than FromPyObject (by making FromPyObject have a __private__ unimplementable method).

@kngwyu
Copy link

kngwyu commented Jan 24, 2020 •
edited
Loading

Thank you for your try to remove specialization here.

We use specialization here to use fast buffer protocol when it's applicable.
However, here's a problem...
Current PyBuffer is not really modified from rust-cpython era, and I feel it has really different semantics from other parts of PyO3.
PyBuffer is not compatible with PyBufferProtocol and rust-numpy.
It apparently needs overhauling.
So I don't want you to put too much effort around buffer protocol here...

In addition: I haven't checked all implementation details... but do we really want to have some specific markers like BufferElement in general FromPyObjectImpl?

@davidhewitt
Copy link
Owner Author

davidhewitt commented Jan 24, 2020 •
edited
Loading

do we really want to have some specific markers like BufferElement in general FromPyObjectImpl?

Not particularly, no. With this design I'm settling towards we may actually be able to remove BufferElement

@davidhewitt davidhewitt reopened this Jan 24, 2020
@davidhewitt
Copy link
Owner Author

Another option I've considered: given this is just an optimisation, we could just disable this optimisation if compiling on stable Rust?

@davidhewitt
Copy link
Owner Author

(close and reopen was an accidental fat finger error)

@kngwyu
Copy link

kngwyu commented Jan 24, 2020 •
edited
Loading

Another option I've considered: given this is just an optimisation, we could just disable this optimisation if compiling on stable Rust?

I think it's OK since rust-numpy is always faster and better as buffer, though we can select some other choices...(e.g, implement to_vec for all types).
Anyway, we don't need to take it seriously now.

davidhewitt pushed a commit that referenced this pull request May 30, 2020
@nagisa @davidhewitt
Proto #1: Exception instances as Py<BaseException>
6fd8787 
This prototype implements use of Py<BaseException> as the instance to
use for exception instances. These instances integrate reasonably well
with the Rust’s standard error mechanisms by implementing the `Display`
and `Error` traits. These error types can also be stored into e.g.
`failure::Fail`s or other error types as a cause of some greater error.
davidhewitt pushed a commit that referenced this pull request May 31, 2020
@nagisa @davidhewitt
Proto #1: Exception instances as Py<BaseException>
a62e18a 
This prototype implements use of Py<BaseException> as the instance to
use for exception instances. These instances integrate reasonably well
with the Rust’s standard error mechanisms by implementing the `Display`
and `Error` traits. These error types can also be stored into e.g.
`failure::Fail`s or other error types as a cause of some greater error.
davidhewitt pushed a commit that referenced this pull request Jul 4, 2020
@nagisa @davidhewitt
Proto #1: Exception instances as Py<BaseException>
78cea1b 
This prototype implements use of Py<BaseException> as the instance to
use for exception instances. These instances integrate reasonably well
with the Rust’s standard error mechanisms by implementing the `Display`
and `Error` traits. These error types can also be stored into e.g.
`failure::Fail`s or other error types as a cause of some greater error.
davidhewitt pushed a commit that referenced this pull request Jul 11, 2020
@nagisa @davidhewitt
Proto #1: Exception instances as Py<BaseException>
8027c3e 
This prototype implements use of Py<BaseException> as the instance to
use for exception instances. These instances integrate reasonably well
with the Rust’s standard error mechanisms by implementing the `Display`
and `Error` traits. These error types can also be stored into e.g.
`failure::Fail`s or other error types as a cause of some greater error.
davidhewitt pushed a commit that referenced this pull request Jul 18, 2020
@nagisa @davidhewitt
Proto #1: Exception instances as Py<BaseException>
496c626 
This prototype implements use of Py<BaseException> as the instance to
use for exception instances. These instances integrate reasonably well
with the Rust’s standard error mechanisms by implementing the `Display`
and `Error` traits. These error types can also be stored into e.g.
`failure::Fail`s or other error types as a cause of some greater error.
@davidhewitt davidhewitt deleted the extract-vec-stable branch August 10, 2021 07:18
davidhewitt pushed a commit that referenced this pull request Dec 4, 2023
@alex
Refactor create_type_object so that most of the code is monomorphic
dd6e033 
In pyca/cryptography this function is the #1 source of lines of generated LLVM IR, because it is duplicated 42x (and growing!). By rewriting it so most of the logic is monomorphic, we reduce the generated LLVM IR for this function by 4x.
davidhewitt pushed a commit that referenced this pull request Jan 2, 2024
@alex @davidhewitt
Refactor create_type_object so that most of the code is monomorphic
1166a99 
In pyca/cryptography this function is the #1 source of lines of generated LLVM IR, because it is duplicated 42x (and growing!). By rewriting it so most of the logic is monomorphic, we reduce the generated LLVM IR for this function by 4x.
davidhewitt pushed a commit that referenced this pull request Oct 29, 2024
@KLMatlock
feat: Adds the Bound<'_, PyMappingProxy> type (PyO3#4644)
0aa13c8 
* Mappingproxy (#1)

Adds in the MappingProxy type.

* Move over from `iter` to `try_iter`.

* Added lifetime to `try_iter`, preventing need to clone when iterating.

* Remove unneccessary borrow.

* Add newsfragment

* Newline to newsfragment.

* Remove  explicit lifetime,

* Review comments (#2)

* Addressing more comments
* Remove extract_bound.
* Remove extract methods.

* Update comments for list return type.

---------

Co-authored-by: Kevin Matlock <kevin.matlock@omicsautomation.com>
davidhewitt pushed a commit that referenced this pull request Oct 30, 2024
@KLMatlock
Mapping returns PyList for keys, values, and items (PyO3#4661)
f74d374 
* Mappingproxy (#1)

Adds in the MappingProxy type.

* Move over from `iter` to `try_iter`.

* Added lifetime to `try_iter`, preventing need to clone when iterating.

* Remove unneccessary borrow.

* Add newsfragment

* Newline to newsfragment.

* Remove  explicit lifetime,

* Review comments (#2)

* Addressing more comments
* Remove extract_bound.
* Remove extract methods.

* Update mapping to return PyList instead of Sequence.

* Update comments for list return type.

* Add newfragment.

* Reimpliment copy with PyMapping type.

* Trigger Build

---------

Co-authored-by: Kevin Matlock <kevin.matlock@omicsautomation.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@davidhewitt @kngwyu