Skip to content

Commit 966ab34

Browse files
VirxECVirxEC
committed
Pass GIL reference instead of re-obtaining lock everytime
Small clippy stuff
1 parent 429fc10 commit 966ab34

4 files changed

Lines changed: 89 additions & 55 deletions

File tree

‎Cargo.lock‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "rlbot-flatbuffers-py"
3-
version = "0.2.0"
3+
version = "0.2.1"
44
edition = "2021"
55
description = "A Python module implemented in Rust for serializing and deserializing RLBot's flatbuffers"
66
repository = "https://github.com/VirxEC/rlbot-flatbuffers-py"

‎build.rs‎

Lines changed: 68 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,12 @@ impl PythonBindGenerator {
6161
};
6262

6363
let has_complex_pack = contents.contains("pub fn pack<'b, A: flatbuffers::Allocator + 'b>(");
64-
let mut file_contents = vec![Cow::Borrowed("use crate::generated::rlbot::flat;")];
64+
let mut file_contents = vec![];
65+
66+
file_contents.push(Cow::Borrowed(match bind_type {
67+
PythonBindType::Struct | PythonBindType::Union => "use crate::{generated::rlbot::flat, FromGil, IntoGil};",
68+
PythonBindType::Enum => "use crate::generated::rlbot::flat;",
69+
}));
6570

6671
if bind_type != PythonBindType::Union {
6772
if has_complex_pack {
@@ -338,6 +343,7 @@ impl PythonBindGenerator {
338343
}
339344

340345
fn generate_enum_definition(&mut self) {
346+
self.write_str("#[allow(non_camel_case_types)]");
341347
self.write_str("#[pyclass(module = \"rlbot_flatbuffers\", get_all, set_all)]");
342348
self.write_str("#[derive(Debug, Default, Clone, Copy)]");
343349
self.write_string(format!("pub enum {} {{", self.struct_name));
@@ -513,9 +519,8 @@ impl PythonBindGenerator {
513519
continue;
514520
}
515521

516-
self.write_string(format!("impl From<{impl_type}> for Py<{}> {{", self.struct_name));
517-
self.write_string(format!(" fn from(mut flat_t: {impl_type}) -> Self {{"));
518-
self.write_str(" Python::with_gil(|py| {");
522+
self.write_string(format!("impl FromGil<{impl_type}> for Py<{}> {{", self.struct_name));
523+
self.write_string(format!(" fn from_gil(py: Python, mut flat_t: {impl_type}) -> Self {{"));
519524
self.write_string(format!(" Py::new(py, {} {{", self.struct_name));
520525

521526
if is_box {
@@ -532,12 +537,11 @@ impl PythonBindGenerator {
532537
let snake_case_name = &variable_info[2];
533538

534539
self.file_contents.push(Cow::Owned(format!(
535-
" {snake_case_name}: flat_t.take_{snake_case_name}().map(Into::into),",
540+
" {snake_case_name}: flat_t.take_{snake_case_name}().map(|x| x.into_gil(py)),",
536541
)));
537542
}
538543

539544
self.write_str(" }).unwrap()");
540-
self.write_str(" })");
541545
self.write_str(" }");
542546
self.write_str("}");
543547
self.write_str("");
@@ -573,21 +577,18 @@ impl PythonBindGenerator {
573577
];
574578

575579
for impl_type in from_impl_types {
576-
self.write_string(format!("impl From<{impl_type}> for Py<{}> {{", self.struct_name));
580+
self.write_string(format!("impl FromGil<{impl_type}> for Py<{}> {{", self.struct_name));
577581

578582
if self.types.is_empty() {
579-
self.write_string(format!(" fn from(_: {impl_type}) -> Self {{"));
580-
self.write_str(" Python::with_gil(|py| {");
581-
self.write_string(format!(" Py::new(py, {} {{}}).unwrap()", self.struct_name));
582-
self.write_str(" })");
583+
self.write_string(format!(" fn from_gil(py: Python, _: {impl_type}) -> Self {{"));
584+
self.write_string(format!(" Py::new(py, {}::default()).unwrap()", self.struct_name));
583585
self.write_str(" }");
584586
self.write_str("}");
585587
self.write_str("");
586588
continue;
587589
}
588590

589-
self.write_string(format!(" fn from(flat_t: {impl_type}) -> Self {{"));
590-
self.write_str(" Python::with_gil(|py| {");
591+
self.write_string(format!(" fn from_gil(py: Python, flat_t: {impl_type}) -> Self {{"));
591592
self.write_string(format!(" Py::new(py, {} {{", self.struct_name));
592593

593594
for variable_info in &self.types {
@@ -601,17 +602,18 @@ impl PythonBindGenerator {
601602
.push(Cow::Owned(format!(" {variable_name}: flat_t.{variable_name},")));
602603
} else {
603604
self.file_contents.push(Cow::Owned(format!(
604-
" {variable_name}: flat_t.{variable_name}.into_iter().map(Into::into).collect(),",
605+
" {variable_name}: flat_t.{variable_name}.into_iter().map(|x| x.into_gil(py)).collect(),",
605606
)));
606607
}
607608
} else if variable_type.starts_with("Option<") {
608609
self.file_contents.push(Cow::Owned(format!(
609-
" {variable_name}: flat_t.{variable_name}.map(Into::into),",
610+
" {variable_name}: flat_t.{variable_name}.map(|x| x.into_gil(py)),",
611+
)));
612+
} else if variable_type.starts_with("Box<") || variable_type.ends_with('T') {
613+
self.file_contents.push(Cow::Owned(format!(
614+
" {variable_name}: flat_t.{variable_name}.into_gil(py),",
610615
)));
611-
} else if Self::BASE_TYPES.contains(&variable_type)
612-
&& !variable_type.starts_with("Box<")
613-
&& !variable_type.ends_with('T')
614-
{
616+
} else if Self::BASE_TYPES.contains(&variable_type) {
615617
self.file_contents
616618
.push(Cow::Owned(format!(" {variable_name}: flat_t.{variable_name},")));
617619
} else {
@@ -622,7 +624,6 @@ impl PythonBindGenerator {
622624
}
623625

624626
self.write_str(" }).unwrap()");
625-
self.write_str(" })");
626627
self.write_str(" }");
627628
self.write_str("}");
628629
self.write_str("");
@@ -636,9 +637,11 @@ impl PythonBindGenerator {
636637
];
637638

638639
for impl_type in from_impl_types {
639-
self.write_string(format!("impl From<&Py<{}>> for {impl_type} {{", self.struct_name));
640-
self.write_string(format!(" fn from(py_type: &Py<{}>) -> Self {{", self.struct_name));
641-
self.write_str(" Python::with_gil(|py| {");
640+
self.write_string(format!("impl FromGil<&Py<{}>> for {impl_type} {{", self.struct_name));
641+
self.write_string(format!(
642+
" fn from_gil(py: Python, py_type: &Py<{}>) -> Self {{",
643+
self.struct_name
644+
));
642645
self.write_str(" let borrow = py_type.borrow(py);");
643646

644647
let is_box_type = impl_type.contains("Box<");
@@ -668,7 +671,7 @@ impl PythonBindGenerator {
668671
let snake_case_name = &variable_info[2];
669672

670673
self.file_contents.push(Cow::Owned(format!(
671-
" {}Type::{variable_value} => flat::{}::{variable_name}(Box::from(borrow.{snake_case_name}.as_ref().unwrap())),",
674+
" {}Type::{variable_value} => flat::{}::{variable_name}(borrow.{snake_case_name}.as_ref().unwrap().into_gil(py)),",
672675
self.struct_name,
673676
self.struct_t_name,
674677
)));
@@ -680,7 +683,6 @@ impl PythonBindGenerator {
680683
} else {
681684
self.write_str(" }");
682685
}
683-
self.write_str(" })");
684686

685687
self.write_str(" }");
686688
self.write_str("}");
@@ -727,18 +729,22 @@ impl PythonBindGenerator {
727729
let is_box_type = impl_type.contains("Box<");
728730

729731
if !is_box_type {
730-
self.write_string(format!("impl From<&{}> for {impl_type} {{", self.struct_name));
732+
self.write_string(format!("impl FromGil<&{}> for {impl_type} {{", self.struct_name));
731733

732734
if self.types.is_empty() {
733-
self.write_string(format!(" fn from(_: &{}) -> Self {{", self.struct_name));
734-
self.write_str(" Self {}");
735+
self.write_string(format!(" fn from_gil(_: Python, _: &{}) -> Self {{", self.struct_name));
736+
self.write_str(" Self::default()");
735737
self.write_str(" }");
736738
self.write_str("}");
737739
self.write_str("");
738740
continue;
739741
}
740742

741-
self.write_string(format!(" fn from(py_type: &{}) -> Self {{", self.struct_name));
743+
self.write_str(" #[allow(unused_variables)]");
744+
self.write_string(format!(
745+
" fn from_gil(py: Python, py_type: &{}) -> Self {{",
746+
self.struct_name
747+
));
742748
self.write_str(" Self {");
743749

744750
for variable_info in &self.types {
@@ -752,17 +758,21 @@ impl PythonBindGenerator {
752758
.push(Cow::Owned(format!(" {variable_name}: py_type.{variable_name},")));
753759
} else {
754760
self.file_contents.push(Cow::Owned(format!(
755-
" {variable_name}: py_type.{variable_name}.iter().map(Into::into).collect(),",
761+
" {variable_name}: py_type.{variable_name}.iter().map(|x| x.into_gil(py)).collect(),",
756762
)));
757763
}
758764
} else if variable_type.starts_with("Option<") {
759765
self.file_contents.push(Cow::Owned(format!(
760-
" {variable_name}: py_type.{variable_name}.as_ref().map(Into::into),",
766+
" {variable_name}: py_type.{variable_name}.as_ref().map(|x| x.into_gil(py)),",
761767
)));
762768
} else if variable_type == "String" {
763769
self.file_contents.push(Cow::Owned(format!(
764770
" {variable_name}: py_type.{variable_name}.clone(),",
765771
)));
772+
} else if variable_type.ends_with('T') || variable_type.starts_with("Box<") {
773+
self.file_contents.push(Cow::Owned(format!(
774+
" {variable_name}: (&py_type.{variable_name}).into_gil(py),",
775+
)));
766776
} else if Self::BASE_TYPES.contains(&variable_type) {
767777
self.file_contents
768778
.push(Cow::Owned(format!(" {variable_name}: py_type.{variable_name},")));
@@ -779,14 +789,16 @@ impl PythonBindGenerator {
779789
self.write_str("");
780790
}
781791

782-
self.write_string(format!("impl From<&Py<{}>> for {impl_type} {{", self.struct_name));
792+
self.write_string(format!("impl FromGil<&Py<{}>> for {impl_type} {{", self.struct_name));
783793

784794
if self.types.is_empty() {
785-
self.write_string(format!(" fn from(_: &Py<{}>) -> Self {{", self.struct_name));
795+
self.write_string(format!(" fn from_gil(_: Python, _: &Py<{}>) -> Self {{", self.struct_name));
786796
self.write_str(" Self::default()");
787797
} else {
788-
self.write_string(format!(" fn from(py_type: &Py<{}>) -> Self {{", self.struct_name));
789-
self.write_str(" Python::with_gil(|py| {");
798+
self.write_string(format!(
799+
" fn from_gil(py: Python, py_type: &Py<{}>) -> Self {{",
800+
self.struct_name
801+
));
790802
self.write_str(" let borrow = py_type.borrow(py);");
791803

792804
if is_box_type {
@@ -806,17 +818,21 @@ impl PythonBindGenerator {
806818
.push(Cow::Owned(format!(" {variable_name}: borrow.{variable_name},")));
807819
} else {
808820
self.file_contents.push(Cow::Owned(format!(
809-
" {variable_name}: borrow.{variable_name}.iter().map(Into::into).collect(),",
821+
" {variable_name}: borrow.{variable_name}.iter().map(|x| x.into_gil(py)).collect(),",
810822
)));
811823
}
812824
} else if variable_type.starts_with("Option<") {
813825
self.file_contents.push(Cow::Owned(format!(
814-
" {variable_name}: borrow.{variable_name}.as_ref().map(Into::into),",
826+
" {variable_name}: borrow.{variable_name}.as_ref().map(|x| x.into_gil(py)),",
815827
)));
816828
} else if variable_type == "String" {
817829
self.file_contents.push(Cow::Owned(format!(
818830
" {variable_name}: borrow.{variable_name}.clone(),",
819831
)));
832+
} else if variable_type.ends_with('T') || variable_type.starts_with("Box<") {
833+
self.file_contents.push(Cow::Owned(format!(
834+
" {variable_name}: (&borrow.{variable_name}).into_gil(py),",
835+
)));
820836
} else if Self::BASE_TYPES.contains(&variable_type) {
821837
self.file_contents
822838
.push(Cow::Owned(format!(" {variable_name}: borrow.{variable_name},")));
@@ -832,8 +848,6 @@ impl PythonBindGenerator {
832848
} else {
833849
self.write_str(" }");
834850
}
835-
836-
self.write_str(" })");
837851
}
838852

839853
self.write_str(" }");
@@ -851,9 +865,9 @@ impl PythonBindGenerator {
851865
}
852866

853867
fn generate_union_new_method(&mut self) {
854-
self.write_str(" #[new]");
855868
assert!(u8::try_from(self.types.len()).is_ok());
856869

870+
self.write_str(" #[new]");
857871
self.write_str(" #[pyo3(signature = (item = None))]");
858872
self.write_string(format!(" pub fn new(item: Option<{}Union>) -> Self {{", self.struct_name));
859873
self.write_str(" match item {");
@@ -928,10 +942,11 @@ impl PythonBindGenerator {
928942

929943
fn generate_struct_new_method(&mut self) {
930944
self.write_str(" #[new]");
945+
self.write_str(" #[allow(clippy::too_many_arguments)]");
931946

932947
if self.types.is_empty() {
933-
self.write_str(" pub const fn new() -> Self {");
934-
self.write_str(" Self {}");
948+
self.write_str(" pub fn new() -> Self {");
949+
self.write_str(" Self::default()");
935950
self.write_str(" }");
936951
return;
937952
}
@@ -1068,7 +1083,7 @@ impl PythonBindGenerator {
10681083
}
10691084

10701085
fn generate_enum_repr_method(&mut self) {
1071-
self.write_str(" pub fn __repr__(&self, _py: Python) -> String {");
1086+
self.write_str(" pub fn __repr__(&self) -> String {");
10721087
self.write_string(format!(" format!(\"{}(value={{}})\", *self as u8)", self.struct_name));
10731088
self.write_str(" }");
10741089
}
@@ -1081,6 +1096,7 @@ impl PythonBindGenerator {
10811096
return;
10821097
}
10831098

1099+
self.write_str("#[allow(unused_variables)]");
10841100
self.write_str(" pub fn __repr__(&self, py: Python) -> String {");
10851101
self.write_str(" format!(");
10861102

@@ -1136,7 +1152,7 @@ impl PythonBindGenerator {
11361152
)));
11371153
} else {
11381154
self.file_contents
1139-
.push(Cow::Owned(format!(" self.{variable_name}.__repr__(py),")));
1155+
.push(Cow::Owned(format!(" self.{variable_name}.__repr__(),")));
11401156
}
11411157
}
11421158

@@ -1152,7 +1168,12 @@ impl PythonBindGenerator {
11521168
} else {
11531169
&self.struct_t_name
11541170
};
1155-
self.write_string(format!(" let flat_t = flat::{name}::from(self);"));
1171+
1172+
if self.bind_type == PythonBindType::Enum {
1173+
self.write_string(format!(" let flat_t = flat::{name}::from(self);"));
1174+
} else {
1175+
self.write_string(format!(" let flat_t = flat::{name}::from_gil(py, self);"));
1176+
}
11561177

11571178
if self.has_complex_pack {
11581179
self.write_str(" let size = flat_t.get_size();");
@@ -1180,9 +1201,9 @@ impl PythonBindGenerator {
11801201
self.write_str(" fn unpack(data: &[u8]) -> Self {");
11811202
self.write_string(format!(" root::<flat::{}>(data).unwrap().into()", self.struct_name));
11821203
} else {
1183-
self.write_str(" fn unpack(data: &[u8]) -> Py<Self> {");
1204+
self.write_str(" fn unpack(py: Python, data: &[u8]) -> Py<Self> {");
11841205
self.write_string(format!(
1185-
" root::<flat::{}>(data).unwrap().unpack().into()",
1206+
" root::<flat::{}>(data).unwrap().unpack().into_gil(py)",
11861207
self.struct_name
11871208
));
11881209
}

‎src/lib.rs‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,30 @@
99
)]
1010
pub mod generated;
1111

12-
#[allow(
13-
clippy::too_many_arguments,
14-
clippy::upper_case_acronyms,
15-
clippy::enum_variant_names,
16-
non_camel_case_types
17-
)]
12+
#[allow(clippy::enum_variant_names)]
1813
mod python;
1914

2015
use pyo3::{prelude::*, PyClass};
2116
use python::*;
2217

18+
pub trait FromGil<T> {
19+
fn from_gil(py: Python, obj: T) -> Self;
20+
}
21+
22+
pub trait IntoGil<T>: Sized {
23+
fn into_gil(self, py: Python) -> T;
24+
}
25+
26+
impl<T, U> IntoGil<U> for T
27+
where
28+
U: FromGil<T>,
29+
{
30+
#[inline]
31+
fn into_gil(self, py: Python) -> U {
32+
U::from_gil(py, self)
33+
}
34+
}
35+
2336
pub trait PyDefault: Sized + PyClass {
2437
fn py_default(py: Python) -> Py<Self>;
2538
}

0 commit comments

Comments
 (0)