Code Review

0x23 · 23 Ocak 2025

Selamlar, çok sevdiğim işlerden biri code review etmek olabilir. Çalıştığım şirkette şu an pek yoğunluğumuz yok bu yüzden review request gelmiyor

yazdığı kodları review ettirmek isteyen arkadaşlar varsa müsait zamanlarımda ücretsiz yapabilirim. Lakin ufak bir iki ricam olucak:

- Alıngan biriyseniz lütfen bunu talep etmeyin.
- Burdaki amaç plugin developer arkadaşların hatalarını görüp kendini geliştirmesidir, benim herhangi bir çıkarım yok, bu yüzden "kasıtlı finding açıyor" demeyin.
- Review github üzerinden yapılacaktır.

"Sen kimsin ve sana neden böyle bir şey yaptıralım?" diye aklında soru işareti olan arkadaşlar için;

-> Bilgisayar Mühendisiyim, şu an yüksek lisans yapıyorum
-> Savunma sanayide çalışıyorum
-> Embedded Linux, Linux System Programming, Modern C++ uzmanlık alanlarım, burdaki "uzmanlık" lafın gelişi değil.
-> Title: Mid-Senior Level C++ Developer

Denizeri24 · 23 Ocak 2025

Engin bilgilerinde Clientte ki CMapOutdoor::GetHeight fonksiyonunu optimize edebilir misin kardeşim;

C++:

float CMapOutdoor::GetHeight(const float fx, const float fy)
{
    const auto fTerrainHeight = GetTerrainHeight(fx, fy);
    constexpr static float CHECK_HEIGHT = 25000.0f;
    float fObjectHeight = -CHECK_HEIGHT;
    const Eigen::Vector3f aVector3d = { fx, -fy, fTerrainHeight };
    FGetObjectHeight kGetObjHeight(fx, fy);
    RangeTester<FGetObjectHeight> kRangeTester_kGetObjHeight(std::move(kGetObjHeight));
    CCullingManager::Instance().PointTest2d(aVector3d, &kRangeTester_kGetObjHeight);
    const auto &temp = kRangeTester_kGetObjHeight.f;

    if (temp.m_bHeightFound) // bottleneck
    {
        fObjectHeight = temp.m_fReturnHeight;
    }

    return std::max<float>(fObjectHeight, fTerrainHeight);
}

-----------

void SpherePack::PointTest2d(const Eigen::Vector3f& p, SpherePackCallback* callback, ViewState state) noexcept
{
    if (state == VS_PARTIAL)
    {
        const auto d = (p - mCenter).eval().head<2>().norm(); // bottleneck - (recursion) // avx2
        const auto radius2 = GetRadius2();

        if (d > radius2) [[likely]]
        {
            return;
        }

        if (radius2 < -d)
        {
            state = VS_INSIDE;
        }
    }

    if (HasSpherePackFlag(SPF_SUPERSPHERE))
    {
        auto pack = mChildren;

        while (pack)
        {
            pack->PointTest2d(p, callback, state);
            pack = pack->_GetNextSibling();
        }
    }
    else
    {
        callback->PointTest2dCallback(p, this, state);
    }
}

Buyur kardeşim bu da performans anazili;

5000 serisi ekran kartı alacağım için mevcut ekran kartımı iade ettim, bu yüzden video kalitesi ve fps i düşük.

0x23 · 23 Ocak 2025

Denizeri24 demiş ki:

Engin bilgilerinde Clientte ki CMapOutdoor::GetHeight fonksiyonunu optimize edebilir misin kardeşim;

C++:

float CMapOutdoor::GetHeight(const float fx, const float fy)
{
    const auto fTerrainHeight = GetTerrainHeight(fx, fy);
    constexpr static float CHECK_HEIGHT = 25000.0f;
    float fObjectHeight = -CHECK_HEIGHT;
    const Eigen::Vector3f aVector3d = { fx, -fy, fTerrainHeight };
    FGetObjectHeight kGetObjHeight(fx, fy);
    RangeTester<FGetObjectHeight> kRangeTester_kGetObjHeight(std::move(kGetObjHeight));
    CCullingManager::Instance().PointTest2d(aVector3d, &kRangeTester_kGetObjHeight);
    const auto &temp = kRangeTester_kGetObjHeight.f;

    if (temp.m_bHeightFound) // bottleneck
    {
        fObjectHeight = temp.m_fReturnHeight;
    }

    return std::max<float>(fObjectHeight, fTerrainHeight);
}

-----------

void SpherePack::PointTest2d(const Eigen::Vector3f& p, SpherePackCallback* callback, ViewState state) noexcept
{
    if (state == VS_PARTIAL)
    {
        const auto d = (p - mCenter).eval().head<2>().norm(); // bottleneck - (recursion) // avx2
        const auto radius2 = GetRadius2();

        if (d > radius2) [[likely]]
        {
            return;
        }

        if (radius2 < -d)
        {
            state = VS_INSIDE;
        }
    }

    if (HasSpherePackFlag(SPF_SUPERSPHERE))
    {
        auto pack = mChildren;

        while (pack)
        {
            pack->PointTest2d(p, callback, state);
            pack = pack->_GetNextSibling();
        }
    }
    else
    {
        callback->PointTest2dCallback(p, this, state);
    }
}

merhaba, öncelikle "kardeşim" hitabı pek hoşuma giden bir hitap şekli değil bunu kullanmamalısınız. bir metin2 developer olmadığım için kodunuza mantıksal ve dil özelinde yorumlarda bulunabilirim, bahsettiğiniz sorunla alakalı genel yapı incelenmeli, debug ve benchmarking çıktıları ele alınmalı.

bunlara ek olarak konuyu dikkatli okumanızı öneririm "gönderin sorununuzu çözeyim, optimize edeyim" diye bir şey yazmıyor, merak eden review isteyen varsa review edebilirim dedim.(bu bir ego değildir, benim de yazdığım kodları review eden ve önerilerde bulunan insanlar var, onların da var bu işler böyledir, yazılım = sürekli öğrenme, farklı bakış açıları vs.)

kodunuzla alakalı;

Kod:

CMapOutdoor::GetHeight:
-> ilk olarak isimlendirmeye dikkat etmenizi öneririm, tabii tek başınıza çalışıyorsanız bu pek bir sorun olmayabilir sizin için
-> move ettiğiniz "kGetObjHeight" variable sadece move edilmek için init edilmiş, burda ek bir maliyetiniz mevcut "kRangeTester_kGetObjHeight" alakalı değerlerle direkt init edilebilir, move gereksiz gibi duruyor
-> move için özel bir nedeniniz varsa move işlemini farklı bir scopeda yapıp amacınızı açıkça belirtmenizi öneririm, moved from statedeki nesne kullanılma tehlikesinde
-> "float fObjectHeight" variable kapsamına dikkat edilmeli, şayet arada bir throw oluşursa boşuna bir allocate maliyetiniz var (fonksiyon noexcept nitelenmemiş bu sebeple throw göz önünde bulundurulmalı, noexcept garantisi verse bile kapsamını daraltmalısınız, bu dikkat edilmesi gereken bir husus)
-> fonksiyon noexcept nitelenebilir, "CCullingManager::Instance().PointTest2d" fonksiyonu noexcept garantisi veriyorsa ki sanıyorum ki altta atmış olduğunuz "SpherePack" classındaki func çalışıyor(tabii inheritance varsa, farklı bir fonksiyonsa duruma göre hareket edilmeli)

Kod:

SpherePack::PointTest2d
-> "const auto d = (p - mCenter).eval().head<2>().norm(); // bottleneck - (recursion) // avx2" burdaki yapmak isteğiniz hesaplama yükü ağır olabilir, alakalı fonksiyonlara göz atılmalı yapılan hesaplama büyüklüğüne göre paralel hesaplama kullanılabilir.
-> farklı bir classda aynı isme ait bir fonksiyon yoksa recursive çağrı kullanmışsınız, pek tavsiye edilen bir durum değil

genel olarak anladığım kadarıyla amacınız yükseklik hesaplamak ve bunla alakalı işlemler yapmak, bunun için multi thread çözümlere gidebilirsiniz. yaşadığınız performans sorunuyla ilgili oyunun yapısıyla ilgilenmediğim için direkt "şundan" diyemeyeceğim büyük ihtimalle etkileyen bir çok faktör mevcuttur, 2 fonksiyon ile buna karar veremem.

Denizeri24 · 23 Ocak 2025

0x23 demiş ki:
merhaba, öncelikle "kardeşim" hitabı pek hoşuma giden bir hitap şekli değil bunu kullanmamalısınız. bir metin2 developer olmadığım için kodunuza mantıksal ve dil özelinde yorumlarda bulunabilirim, bahsettiğiniz sorunla alakalı genel yapı incelenmeli, debug ve benchmarking çıktıları ele alınmalı.

bunlara ek olarak konuyu dikkatli okumanızı öneririm "gönderin sorununuzu çözeyim, optimize edeyim" diye bir şey yazmıyor, merak eden review isteyen varsa review edebilirim dedim.(bu bir ego değildir, benim de yazdığım kodları review eden ve önerilerde bulunan insanlar var, onların da var bu işler böyledir, yazılım = sürekli öğrenme, farklı bakış açıları vs.)

kodunuzla alakalı;

Kod:

CMapOutdoor::GetHeight: -> ilk olarak isimlendirmeye dikkat etmenizi öneririm, tabii tek başınıza çalışıyorsanız bu pek bir sorun olmayabilir sizin için -> move ettiğiniz "kGetObjHeight" variable sadece move edilmek için init edilmiş, burda ek bir maliyetiniz mevcut "kRangeTester_kGetObjHeight" alakalı değerlerle direkt init edilebilir, move gereksiz gibi duruyor -> move için özel bir nedeniniz varsa move işlemini farklı bir scopeda yapıp amacınızı açıkça belirtmenizi öneririm, moved from statedeki nesne kullanılma tehlikesinde -> "float fObjectHeight" variable kapsamına dikkat edilmeli, şayet arada bir throw oluşursa boşuna bir allocate maliyetiniz var (fonksiyon noexcept nitelenmemiş bu sebeple throw göz önünde bulundurulmalı, noexcept garantisi verse bile kapsamını daraltmalısınız, bu dikkat edilmesi gereken bir husus) -> fonksiyon noexcept nitelenebilir, "CCullingManager::Instance().PointTest2d" fonksiyonu noexcept garantisi veriyorsa ki sanıyorum ki altta atmış olduğunuz "SpherePack" classındaki func çalışıyor(tabii inheritance varsa, farklı bir fonksiyonsa duruma göre hareket edilmeli)

Kod:

SpherePack::PointTest2d -> "const auto d = (p - mCenter).eval().head<2>().norm(); // bottleneck - (recursion) // avx2" burdaki yapmak isteğiniz hesaplama yükü ağır olabilir, alakalı fonksiyonlara göz atılmalı yapılan hesaplama büyüklüğüne göre paralel hesaplama kullanılabilir. -> farklı bir classda aynı isme ait bir fonksiyon yoksa recursive çağrı kullanmışsınız, pek tavsiye edilen bir durum değil

genel olarak anladığım kadarıyla amacınız yükseklik hesaplamak ve bunla alakalı işlemler yapmak, bunun için multi thread çözümlere gidebilirsiniz. yaşadığınız performans sorunuyla ilgili oyunun yapısıyla ilgilenmediğim için direkt "şundan" diyemeyeceğim büyük ihtimalle etkileyen bir çok faktör mevcuttur, 2 fonksiyon ile buna karar veremem.

Raviel · 23 Ocak 2025

Selamlar,
Örnek bir sistemi ele alarak veya forumda ilginizi çeken herhangi bir konu varsa bunun üzerinden bir code review yapabilir misiniz? Hem bu şekilde biraz daha insanlara yol gösterici olabilir

Metin2 developerı değilseniz bile oyuna olan ilginiz sizi bu foruma getirmiş olmalı. Ayrıca konu adına da teşekkürler bence bilgilendirici bir konu olabilir.

Tengrist · 23 Ocak 2025

Gayretiniz için teşekkürler, bence oyunun kendi cube sistemini incelerseniz nasıl bir rezaletle karşı karşıyayız görebiliriz. Belki foruma da paylaşabilirsiniz

0x23 · 23 Ocak 2025

Denizeri24 demiş ki:
21405 eklentisini görüntüle

/////

Raviel demiş ki:
Selamlar,
Örnek bir sistemi ele alarak veya forumda ilginizi çeken herhangi bir konu varsa bunun üzerinden bir code review yapabilir misiniz? Hem bu şekilde biraz daha insanlara yol gösterici olabilir Metin2 developerı değilseniz bile oyuna olan ilginiz sizi bu foruma getirmiş olmalı. Ayrıca konu adına da teşekkürler bence bilgilendirici bir konu olabilir.

selamlar, teşekkür ederim evet 1-2 aydır falan ara ara source dosyalarına bakıyorum tabii aşırı hakimiyetim yok metin2 üzerinde. Yani sistemleri neye göre kategorize ederim bilemedim şayet önerebileceğiniz varsa inceleyebilirim.

Tengrist demiş ki:
Gayretiniz için teşekkürler, bence oyunun kendi cube sistemini incelerseniz nasıl bir rezaletle karşı karşıyayız görebiliriz. Belki foruma da paylaşabilirsiniz

ben teşekkür ederim, boş vakitlerde farklı alanlarla ilgilenmek güzel oluyor, insana bazen yarıyor böyle şeyler. not aldım, inceledikten sonra müsait bir anımda cube sistemiyle alakalı bir konu açabilirim detaylı

pasha37 · 23 Ocak 2025

0x23 demiş ki:
-> "float fObjectHeight" variable kapsamına dikkat edilmeli, şayet arada bir throw oluşursa boşuna bir allocate maliyetiniz var (fonksiyon noexcept nitelenmemiş bu sebeple throw göz önünde bulundurulmalı, noexcept garantisi verse bile kapsamını daraltmalısınız, bu dikkat edilmesi gereken bir husus)

stack üzerinde alan kullanan local değişkenin nasıl bir maliyeti var kardeşim?

Denizeri24 · 23 Ocak 2025

pasha37 demiş ki:
stack üzerinde alan kullanan local değişkenin nasıl bir maliyeti var kardeşim?

sormuş gpt ye almış cevap yazmış buraya :s17:

pasha37 · 23 Ocak 2025

verilen kodda nerede heapden alan açıldığını gösterir misin üstad

0x23 · 23 Ocak 2025

pasha37 demiş ki:
verilen kodda nerede heapden alan açıldığını gösterir misin üstad

verilen kodda değil, sizin “koray” nickiyle farklı bir forumda paylaştığınız “anti cheat” kodunda vardı. cümle “yaptığınızı hatırlıyorum” diyor , dikkatli okumanızı tavsiye ederim.

///

pasha37 · 23 Ocak 2025

0x23 demiş ki:
verilen kodda değil, sizin “koray” nickiyle farklı bir forumda paylaştığınız “anti cheat” kodunda vardı. cümle “yaptığınızı hatırlıyorum” diyor , dikkatli okumanızı tavsiye ederim.

konuyla alakası nedir onu bile geçtim alıntıladığın mesajda sorduğum ile alakası nedir? daha stack alanının maliyet oluşturacağını düşünüyorsan demagoji yerine hizmet sunduğun alanda kendini geliştirmeni tavsiye ederim

0x23 · 23 Ocak 2025

pasha37 demiş ki:
konuyla alakası nedir onu bile geçtim alıntıladığın mesajda sorduğum ile alakası nedir? daha stack alanının maliyet oluşturacağını düşünüyorsan demagoji yerine hizmet sunduğun alanda kendini geliştirmeni tavsiye ederim

hocam hayatınızda hiç gömülü sistem yazdınız mı? bare metalde kod geliştirdiniz mi? 512Kb RAM+ROM üzerinde koşacak servis kodladınız mı? kendi allocaterınızı yazdınız mı? oyun OS üzerinde, ramin kısıtlı olmadığı(gömülü alana nazaran) bir ortamda koştuğundan bu maliyet sizin için anlamsız olabilir, C++ ile üretim yapıp bahsettiğim noktalara dikkat etmeyen bir şirket bulamazsınız kolay kolay, alanımda bir sorunum olsa çalıştığım yer babamın mı ki bana aselsan, roketsan vb. firmalarda yöneticilik yapmış meslektaşlarım title versin, oturup bunu bir düşünün.

pasha37 · 23 Ocak 2025

0x23 demiş ki:
hocam hayatınızda hiç gömülü sistem yazdınız mı? bare metalde kod geliştirdiniz mi? 512Kb RAM+ROM üzerinde koşacak servis kodladınız mı? kendi allocaterınızı yazdınız mı? oyun OS üzerinde, ramin kısıtlı olmadığı(gömülü alana nazaran) bir ortamda koştuğundan bu maliyet sizin için anlamsız olabilir, C++ ile üretim yapıp bahsettiğim noktalara dikkat etmeyen bir şirket bulamazsınız kolay kolay, alanımda bir sorunum olsa çalıştığım yer babamın mı ki bana aselsan, roketsan vb. firmalarda yöneticilik yapmış meslektaşlarım title versin, oturup bunu bir düşünün.

yıl olmuş 2025 parmak kadar sd cardlar bile son kullanıcı için 2tb seviyesine gelmiş ve yaygınlaşmış, min 16mb ramli espler bile son kullanıcıya 2-3 dolara satılır seviyedeyken o kadar kısıtlı donanımla çalışman benim problemim değil, konu başlığında belirttiğin "modern" dillerin "modern" standartlarına ayak uyduramayacaksan c ile devam edebilirsin.

ve hala bahsettiğin şeylerin hiçbiri stack üzerinden alan maliyeti oluşturacağını sanmak için geçerli bir sebep değil. fonksiyon sonlandığında alanda sonlanacak ve maliyetini savunduğun şey ise 4b bir değişken, geçiniz...

0x23 · 23 Ocak 2025

pasha37 demiş ki:
yıl olmuş 2025 parmak kadar sd cardlar bile son kullanıcı için 2tb seviyesine gelmiş ve yaygınlaşmış, min 16mb ramli espler bile son kullanıcıya 2-3 dolara satılır seviyedeyken o kadar kısıtlı donanımla çalışman benim problemim değil, konu başlığında belirttiğin "modern" dillerin "modern" standartlarına ayak uyduramayacaksan c ile devam edebilirsin.

ve hala bahsettiğin şeylerin hiçbiri stack üzerinden alan maliyeti oluşturacağını sanmak için geçerli bir sebep değil. fonksiyon sonlandığında alanda sonlanacak ve maliyetini savunduğun şey ise 4b bir değişken, geçiniz...

bir gün savunma sanayide çalışırsanız görürsünüz requirementlardan dolayı mı böyle yoksa keyfi mi diye, gider dersiniz “sd kartlar bile 2tb ya”. savunduğum şeyin kapsamların düzgün kullanılmasına dair bir best practies olduğunu anlamanızda bir sorun mevcut, siz modern c++ diyip fonksiyon scopeunda localde heap kullanmaya falan devam edin, yazdığınız kodlarda belli oluyor zaten bazı şeyler. ben bu ülkenin en tepesindeki firmalarda çalışmış meslektaşlarımla konuştuğum, öğrendiğim ve öğrenmeye devam ettiğim yöntemlerle ilerleyeceğim, laf yetiştirmek yerine bu “derin” bilgilerinizi forum üyeleri yararına kullanmanız dileğiyle.

pasha37 · 23 Ocak 2025

0x23 demiş ki:
bir gün savunma sanayide çalışırsanız görürsünüz requirementlardan dolayı mı böyle yoksa keyfi mi diye, gider dersiniz “sd kartlar bile 2tb ya”. savunduğum şeyin kapsamların düzgün kullanılmasına dair bir best practies olduğunu anlamanızda bir sorun mevcut, siz modern c++ diyip fonksiyon scopeunda localde heap kullanmaya falan devam edin, yazdığınız kodlarda belli oluyor zaten bazı şeyler. ben bu ülkenin en tepesindeki firmalarda çalışmış meslektaşlarımla konuştuğum, öğrendiğim ve öğrenmeye devam ettiğim yöntemlerle ilerleyeceğim, laf yetiştirmek yerine bu “derin” bilgilerinizi forum üyeleri yararına kullanmanız dileğiyle.

engin bilgi ve tecrübelerinizle selçuk bey içinde bir code review gelir mi üstad?

Black Jap · 23 Ocak 2025

Cevap vermek haddim değil ama sözde uzmanlık alanınız savunma sanayisinde calısmaksa metin2 gibi keyif geliştiriciliği yapılan bir alana neden yazıyorsunuz üstelik bilgim yok kodlama mantığı ile, cevap vericem diyorsunuz metin2 kütüpheneleri hakkında bilginiz yoksa fikriniz de olmasın insanları da yargılamayın

0x23 · 23 Ocak 2025

pasha37 demiş ki:
engin bilgi ve tecrübelerinizle selçuk bey içinde bir code review gelir mi üstad?

her proje aynı mı? SIHA tek bir modülden falan mı oluşuyor? oluşturulan char arrayden ne çıkardınız? insanlara yardımcı olmaya çalışmam sizi neden geriyor? gönderdiğinizle benim söylediğim ne alaka? cahillikle savaşmak harbiden çok yorucu, hiçbir şey bilmeyip her şeyi bildiğinizi sanmak size uzun vadede zarar verir. size ayıracak daha fazla değersiz vaktim yok, kendi ütopyanızda takılın, konuyuda kirletmeyin. bol şans.

Raviel · 23 Ocak 2025

Forum yöneticisi olmadığından bu konuya karışmam doğru değil ancak burası bir forum sitesi herkes bilgilerini paylaşmakta özgürken yapılan yorumlar hoş değil. Ayrıca konuyu açan arkadaşımız kendi eğitimini ve gerekli konumunu belirterek tecrübelerini paylaşmak istemişken sizlerin bu şekilde yorum yapmaktansa düzgünce düşüncelerinizi ifade ederseniz bence sorun kalmaz ancak onun yerine böyle agresif yorumlarla yeni insanları soğutur sadece sektörde ki bilgi akışını kısıtlarsınız.

WILDGUARD · 23 Ocak 2025

Konuya işlem yapıyorum, işlem yaptığım yorumlara bildirim atıyorum fakat kimsenin umrunda değil. Arkadaşlar tartışma yaratmak, konu dışı yorum atmak kural ihlalleridir. Konu ihlallerle dolu olduğu için açılmamak üzere kilitlenmiştir. Bu gibi yorumlar farklı konularda devam edilirse hangi üye olursa olsun tartışma yaratmaktan forumdan uzaklaştırılacaktır

Paylaşım platformunda olduğunuzu unutmayın, hünerlerinizi yardım arayan insanlar için gösterin birbirinize değil.

Code Review

Çaylak Üye

Gentleman

Çaylak Üye

Gentleman

Premium Üye

Yardımsever Üye

Çaylak Üye

Üye

Gentleman

Üye

Çaylak Üye

Üye

Çaylak Üye

Üye

Çaylak Üye

Üye

Üye

Çaylak Üye

Premium Üye

//

Benzer konular