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

Remove stl extensions #59

Open
jazzvaz opened this issue Dec 28, 2017 · 22 comments
Open

Remove stl extensions #59

jazzvaz opened this issue Dec 28, 2017 · 22 comments

Comments

@jazzvaz
Copy link
Contributor

jazzvaz commented Dec 28, 2017

Stl allocator uses new operator, which is already overloaded and pointing to xr memory. Therefore, both xr and stl allocators using same memory resources.

@Xottab-DUTY
Copy link

So, the task is to remove xr_new and xr_delete and such?)

@jazzvaz
Copy link
Contributor Author

jazzvaz commented Dec 28, 2017

No, the task is to remove xr containers and make them as normal stl containers. I see them as a type redefinition. For example, xr_vector is exactly the same as std::vector. However, xr_vector uses custom allocator (xalloc) to get memory from xr_memory, while stl uses allocator which calls new operator, and new operator is already being overloaded and pointing to xr_memory. Hopefully, now it is more clear :)

@Xottab-DUTY
Copy link

new operator overloading is defined in the xrMemory.h
The problem is that you must include xrMemory.h BEFORE the STL files so that our new operator overloading can be overloaded for STL

@jazzvaz
Copy link
Contributor Author

jazzvaz commented Dec 28, 2017

@Xottab-DUTY I tested it in one of my projects, and it does not matter when you include a header with implementation of a new operator, it will always work. Even when the program starts, any allocation is going through my new operator. However, I tested it only with static linking, and don't know the behavior with dynamic linking.
The problem with xr is that xr_memory should become singleton or be initialized before any call to it. Another solution: initialization inside the constructor.

@ForserX
Copy link

ForserX commented Dec 28, 2017

xr_delete не советую трогать. В движке не всегда переменные получают значения, придется проверять на их наличие.

@jazzvaz
Copy link
Contributor Author

jazzvaz commented Dec 28, 2017

xr_delete и не придется трогать, только контейнеры xr_vector, xr_string...

@Xottab-DUTY
Copy link

Ахаха. Спрашивается, а что же это мы, хлопцы, на английском-то разговариваем, коли все тут по-русски балакають?))

Хотя, конечно, понятно зачем)

@jazzvaz
Copy link
Contributor Author

jazzvaz commented Dec 28, 2017

Чтобы всем было понятно :)

@Im-dex
Copy link
Owner

Im-dex commented Dec 28, 2017

Global operator new overloading is pretty simple, you should define it in one of the TUs and that's all, there is no need to include its definition into every TU.

@ForserX

xr_delete не советую трогать. В движке не всегда переменные получают значения, придется проверять на их наличие.

Он только на null проверяет же, а это делает и стандартный delete, только видимо в gsc никто об этом не знал

@jazzvaz
Copy link
Contributor Author

jazzvaz commented Dec 28, 2017

xr_delete, xr_new, xr_special... в принципе они все могут быть убраны и заменены обычным new и delete, так как они просто повторяют их работу.

@ForserX
Copy link

ForserX commented Dec 28, 2017

@Im-dex, да, только с учётом перегрузок delete в xray, я ловил исключения, когда делал отказ от xr_delete. Поэтому сейчас частично ушёл от xr_new и только.

@jazzvaz
Copy link
Contributor Author

jazzvaz commented Dec 29, 2017

скорее всего new и delete сломаны так как они в хедере,перенеся их в имплеменатцию, я получаю совсем другое поведение. И это все из за того что кто решил не смотреть на предупреждения и написать#pragma warning(disable : 4595)

@Im-dex
Copy link
Owner

Im-dex commented Dec 29, 2017

@jazzvaz если переносить в цппшник, то нужно убирать inline + это должно быть сделано для каждого dll проекта
PS: и еще про ODR нужно не забыть

@jazzvaz
Copy link
Contributor Author

jazzvaz commented Dec 29, 2017

@Im-dex это понятно на счет inline. Для каждого dll проекта нужен operator new?

@Im-dex
Copy link
Owner

Im-dex commented Dec 29, 2017

@jazzvaz Да, в каждом нужно переопределять, иначе дефолтный будет использоваться

@jazzvaz
Copy link
Contributor Author

jazzvaz commented Dec 29, 2017

@Im-dex еще один плюс к статичным библиотекам

@Im-dex
Copy link
Owner

Im-dex commented Dec 29, 2017

@jazzvaz Ну да, в этом плане с ними все сильно проще

@ForserX
Copy link

ForserX commented Dec 29, 2017

А чем плох дефолт для рентгена? Не у ПК, переопределения не помню.

@jazzvaz
Copy link
Contributor Author

jazzvaz commented Dec 29, 2017

@ForserX дефолтный идет к малоку за памятью, а не к xr memory у которого своя система распределения памяти

@Xottab-DUTY
Copy link

У нас в OpenXRay tamlin-mike вынес new, delete в отдельную статическую библиотеку. Честно говоря, я не проверял, но, вроде бы, оно таким образом переопределяется везде....

Надо будет проверить....

@jazzvaz
Copy link
Contributor Author

jazzvaz commented Dec 29, 2017

@Xottab-DUTY можно ссылку пожалуйста или какой иммено репозиторий?

@Xottab-DUTY
Copy link

Xottab-DUTY commented Dec 29, 2017

@jazzvaz мой форк OpenXRay. Либо в оригинальном репозитории можешь посмотреть ветку speedup.

Добавлено позже:
Оригинальный коммит в форке tamlin-mike.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants