1. Java / Говнокод #11301

    +83

    1. 01
    2. 02
    3. 03
    4. 04
    5. 05
    6. 06
    7. 07
    8. 08
    9. 09
    10. 10
    11. 11
    12. 12
    13. 13
    14. 14
    15. 15
    16. 16
    17. 17
    18. 18
    19. 19
    20. 20
    21. 21
    22. 22
    23. 23
    24. 24
    25. 25
    26. 26
    27. 27
    28. 28
    29. 29
    private String toHTML(String unicode)
        {
            String output = "";
    
            char[] charArray = unicode.toCharArray();
    
            for (int i = 0; i < charArray.length; ++i)
            {                        
                if ((int)charArray[i]>255)
                {
                    String s = ""+Integer.toHexString(charArray[i]);
                    switch (s.length())
                    {
                        case 1: s="\\u000"+s; break;
                        case 2: s="\\u00"+s; break;
                        case 3: s="\\u0"+s; break;
                        case 4: s="\\u"+s; break;
                        default: throw new RuntimeException( s +" is tool long to be a Character");
                    }
                    output += s;
                }
                else
                {
                    output += charArray[i];
                }
                
            }
            return output;
        }

    Эпичнейший говнокод! На серваке top показывает нагрузку 10-12. 3000 пользователей, 100 нод, интеграция с SAP, который пачками проводит документы и выдаёт цены, отчёты по остаткам и т.п. И всё это, как оказалось, капля в море по сравнению с 5 человеками техподдержки, которые сидят в аяксовой консоле мониторинга, для которой HTTP-ответ экранируется данным шедевром. Без этого шедевра нагрузка держится в районе 2-3 даже при достаточно большой активности.

    Запостил: konsoletyper, 26 Июня 2012

    Комментарии (78) RSS

    • > for (int i = 0; i < charArray.length; ++i)
      > output += charArray[i]
      Бедный GC... за что они его так...
      Ответить
      • Теоретически тут может escape analysis разрулить ситуацию. Но всё равно, реализация java.lang.String ужасается от такого надругательства над собой.
        Ответить
        • > Теоретически тут может escape analysis разрулить ситуацию.
          А куда ему деваться? Стринг билдер или его аналог он всяко не додумается вставить. Так и будет конкатенировать до посинения ;)
          Ответить
          • Правильно, будет. Но в стеке. Стек будет пыхтеть, а GC даже не заметит :-)
            Ответить
            • Простите, а как конкатенировать строки в стеке?
              Ответить
            • Не хотелось бы получить документ в 1Mb на стеке.
              Ответить
            • Мне кажется, в стеке ничего не будет пыхтеть. Я конечно не уверен, но я сомневаюсь, что Java умеет схлапывать всю иерархию объектов в стек. СтрингБилдер скорей всего реализован как ссылка на внутренний расширяемый массив, который выделяется в куче. Т.е. при escape analysis в стек положить осилит только shallow-уровень полей, т.е. внутренний заголовок объекта, ссылку на внутренний объект и число символов. И всё.
              Ответить
              • И строки в Java реализованы так же, как и StringBuilder -- просто указатель на char[], разница только в мутабельности.
                Ответить
      • Я в Дельфах часто строки по 1 символу наращиваю.
        Там амортизация вшита, если что.
        Ответить
        • Они мутабельные?
          Ответить
          • Да
            Ответить
            • Плохой дизайн ведь. Строки иммутабельными должны быть.
              Ответить
              • Почему?
                Ответить
                • Допустим, захотел ты использовать ассоциативный массив String -> MyObject. Положил в него по ключу-строке "ok" какой-то объект. А потом кто-то добрый решил дописать символов в строку-объект, чтобы получить "okay", чего зря символам пропадать. Если ты захочешь объект свой обратно получить, то вот беда, по "ok" ты до него не достучишься. Более того, если твой массив - это хэш-таблица, скорее всего ты вообще свой объект ни по какому ключу не получишь.
                  Выход, конечно, есть - копировать строки при использовании их в качестве ключей. Проблема в том, что копирование будет происходить даже в том случае, если строки никто менять не собирался.
                  Более изящний выход - сделать строки иммутабельными, тогда и копировать ничего не нужно, и сломать массив никто не сможет. А чтобы собирать новые строки, нужно использовать буфер в явном виде.
                  Второй вариант безопаснее, особенно в контексте средней квалификации программистов на жабе.
                  Ответить
                  • > А потом кто-то добрый решил дописать символов в строку-объект

                    Ну допишет, и как это скажется на ассоциативном массиве? В Дельфах коровьи строки ващето. А в крестах std::string вообще копируется на каждое =, и тоже мутабелен, какой ужас.
                    Ответить
                    • > копируется на каждое =
                      Ну там всего скорее copy on first write.
                      А поддержание такого механизма в многопоточной среде сложнее и рискованней, чем тупо сделать строки иммутабельными. Да и реализовать такой String на чистой жабе (там только один нативный метод - intern, появившийся относительно недавно) не получилось бы, перегрузки операторов нет.
                      Ответить
                      • никаких copy on first write, всё по-честному
                        именно из-за
                        > поддержание такого механизма в многопоточной среде
                        ну и из-за того, что ты всегда можешь залезть грязными руками в содержание строки и чего-нибудь да поменять, никакой COW механизм это не отследит

                        если так хочется хардкора (а для больших строк периодически хочется), то тупо boost::shared_ptr<some_big_object> и все счастливы
                        по факту спасает непосредственно то, что это какая то спарта передавать string & дальше в обработку, ведь в 95% случаев строка создается один раз и возится по коду через const &
                        Ответить
                        • > никаких copy on first write
                          Собственно, вернулись в исходную точку. Получается, что мапы, к примеру, должны делать копии ключей, даже когда это не нужно.

                          Кстати, от иммутабельный строк есть ещё один бенефит в контексте ассоциативных массивов - для них можно кэшировать значение хэш-функции, что позволит рассчитывает его один раз на объект.
                          Ответить
                          • собственно, палка о двух концах - где кроме мапа нужно еще хранить все эти ключи?
                            ну и т.к. unmanaged code - хочу в своем потоке копирую строку, хочу меняю - остальные потоки это никак не затрагивает, строка вообще может быть на стеке (в реализации msvs строки до 16 символов живут внутри объекта, а не в куче) или в TLS - никакой гц никто за ниточки не дергает

                            а хеш по строке считается не так медленно, чтобы ради этого настолько сильно переживать
                            в любом случае, в с++ невозможно будет красиво его держать в актуальном состоянии - та же проблема, что и с COW - строки-то мутабельные - вася пупкин делает str[13] = 'ъ', а петя жопкин вообще берёт и теребит свой (char *)&str[0], и всё, давай до свидания - никто не отследит момент, когда хеш пора перерасчитывать
                            Ответить
                            • я не говорю что иммутабельные строки плохо
                              всё упирается в наличие или отсутствие управляемой среды
                              есть - значит можно фантазию проявлять, глобально считать использование объектов, запрещать запись туда, где нельзя и т.д.
                              нет - ну что ж поделать, пистолет дали, крутись как хочешь
                              Ответить
                            • петя жопкин лох в любом случае, т.к. std::string не гарантирует непрерывность данных в памяти. Но это так, к слову.
                              Ответить
                              • к слову гарантирует по с++11 и всегда гарантировал по факту (ни один вендор так и не удосужился сделать std::string прерывным в памяти)
                                Ответить
                          • >Получается, что мапы, к примеру, должны делать копии ключей, даже когда это не нужно.
                            Вон иммутабильная строка в 100500 символов и используется только в одном месте кода, а потом вдруг решил её поменять немного для использования в том же самом месте и делаешь лишнюю копию всей длиннющей строки в 100500 символов. Хватит с этими двойными стандартами. Вон вроде строка иимутабильная, а копию лишнюю делаешь.
                            Ответить
                            • Редкий случай же, не показательный. А вот крестушка на каждом шагу каждое значение по пятьдесять раз перекопирывоывает.
                              Ответить
                            • >>Получается, что мапы, к примеру, должны делать копии ключей, даже когда это не нужно.
                              Так это при мутабельных строках.

                              >Вон вроде строка иимутабильная, а копию лишнюю делаешь.
                              Здесь еще одна фишка есть - за любые изменения иммутабельной строки платит только тот, кто ее меняет. Но за возможность менять мутабельную строку платят все, кто ее юзает ее копии. Видимо разработчики явы оценили, какие случаи чаще - и сделали строки иммутабельными.
                              Ответить
                            • Со строками это редко нужно.
                              Однако есть другой иммутабельный тип, для которого эта особенность стреляет - BigInteger/BigDecimal. Вроде делать число мутабельным - грех, а на больших числах производительность из-за иммутабельности серьёзно проседает.
                              Ответить
                            • Тут вообще имхо правильно поступили в жабе.
                              Хочешь юзай немутабельные Стринги, хочешь мутабельный StringBuffer (причем изначально в JDK 1.0 сделали они потокобезопастным).

                              И только гораздо позже из практических соображений добавили thread-unsafe StringBuilder.

                              Тут расчет на то что кодер, он не тупая обезьянка и знает чего хочет. Где ему выгодней немутабельное - он будет юзать его.
                              В пользу того что решения кстати говорит и то что в шарпе полностью переняли подход.
                              Ответить
                            • А вот в языках с принудительной немутабельностью, типа Хацкила, там программера держат за дурака: не переживай, чувак - Весь мусор за тебя соберем.
                              За ссылки не парься - объекты немутабельны - хер поменяешь.
                              Треды? Да всё ништяк. Оно само распаралелится. Итд.


                              Вот в таком случае (когда компилятор и среда исполнения предполагаются умнее чем программист) то этими проблемами и должны заниматься они.
                              Как я описал ниже VM должна быть умной:
                              - видишь что применением функции из одного объекта получаем другой, а этот уже никому не нужен - не выделяй память, а модифицируй по возможности старый и подсунь ссылку на него.
                              Ответить
                              • > а модифицируй по возможности старый
                                это если длина совпадает байт-в-байт. собственно, предполагаю, все современные VM так и делают.
                                Ответить
                                • >если длина совпадает байт-в-байт
                                  Нет, я говорю о самом общем случае мутации.
                                  Вот взять этот пример:
                                  for (){
                                  output += x;
                                  }
                                  Единственная ссылка на старый output удаляется и заменяется ссылкой на новый объект строки.
                                  Если компилятор видит такую ситуацию - он ставит в байт-коде на типе строки какой-то хинт типa Mutable на CharArray и выделяет память с избытком (например 2^n).Тогда в строку можно добавлять данные спереди и сзади (надо только дополнительно хранить позицию начала) и при переполнении расширять до 2^(n+1).
                                  И при любых мутациях - append, prepend, delete, write (разве что кроме inserta) мы просто меняем и снова используем старый объект и даже ссылку на output менять не надо.

                                  Возможно строки в яве - не очень удачный пример, но вот в функциональных языках где немутабельные объекты то и дело неявно создаются и собираются GC такого рода оптимизации уже неиспользуемых обектов сделать вполне реально.
                                  Ответить
                        • Ну в Дельфи тоже можно поменять символы строки в обходы коровы, ещё можно поменять счётчик ссылок и ещё много вещей, но это не повод отказыватся от удобств. Кто кулхацкерит - тот сам пусть думает о последствиях.
                          Ответить
                          • Все верно. В Qt вон строки тоже с коровкой, и никто не лезет к ним напрямую.
                            Ответить
                • Потокобезопасность (можно спокойно передать строку в другой поток без копирования данных).
                  Очень шустрые substr'ы.
                  Экономия памяти на одинаковых строках и их кусках без накладных расходов на копирование или подсчет ссылок.
                  Достаточно быстрая конкатенация (за O(длины результата) если делать ее не через жопу).
                  Ответить
                  • > Очень шустрые substr'ы
                    Тут и downside есть: вытянул документище в память, взял у него substr(x, x+5), и выбросил. А ссылка на документище всё ещё болтается в этом substr'е. Но это не так часто бывает и довольно легко решается.
                    Ответить
                    • Ну downside'ов у них тоже хватает. Например иммутабельная строка плохо подходит для множества мелких изменений (например, как в коде ОПа замена символов на \uxxxx).

                      Ничего идеального в этом мире нет ;(
                      Ответить
                      • В треде всё все хорошо и правильно говорили.
                        И не хотел влезать, но.
                        >Тут и downside есть: вытянул документище в память, взял у него substr(x, x+5), и выбросил.
                        >А ссылка на документище всё ещё болтается в этом substr'е.

                        Возможно это пахнет фантастикой, но ведь можно сделать так чтобы GC освобождал страницы памяти с неиспользуемыми более кусками массивов.
                        То есть считать не только недоступные объекты, но и страницы (в случае больших массивов). Это кстати поможет не только подстрокам, но подлистам на основе массивов.

                        То есть это проблемы сборщика - что он тупой недостаточно умный.

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

                        Это касается того случая когда мы видели адские цифры на permutationах в Хацкиле, а ведь будь оно достаточно умным - могло бы просто модифицировать старый список.
                        Опять-таки это недостаточно умная среда исполнения, а не проблема иммутабельных объектов.
                        Ответить
                        • > Возможно это пахнет фантастикой, но ведь можно сделать так чтобы GC освобождал страницы памяти с неиспользуемыми более кусками массивов.
                          В хаскеле вроде бы так lazy bytestring сделан. Строка записана в виде списка иммутабельных сегментов, которые могут подкачиваться по требованию и уничтожаться GC по частям, если эти части не нужны.
                          Ответить
                          • Ну вот. Я не шарю как оно в Хацкиле, но если принуждают к немутабельности то так по логике вещей и должно быть :)
                            Ответить
                          • круто.
                            а то в той же жабе полно западла:
                            String a="aaaaaaaaaaaaaaaaaaaaaaaaaaaa"; //в общем мегадлинная строка откуда-то
                            a=a.substring(a.length()-1,1); // a по-прежнему указывает на тот же объект
                            System.out.println(a); // a
                            и вот тут многие думают, что сборщик сможет собрать мегадлинную строку, оставив маинькую субстроку - а нифига
                            a=new String(a.substring(a.length()-1,1));
                            вот только так всё будет хорошо
                            Ответить
                • >Почему?
                  В большинстве приложений строки функционально это базовый примитивный тип, который на практике имеет семантику by value. Прикинь, что было бы, если бы, скажем, число Integer всегда было ссылочным. Какой-то поток в каком-то месте изменил число с 7 на 1 -- и это везде отразилось во всех участках кода, куда число затесалось. А отдебагить сложно, куда затесалось, куча геморроя. Поэтому разумно по умолчанию делать такие объекты неизменяемыми. Этакая симуляция byvalue в контексте иерархии ссылочных объектов.
                  Ответить
                  • А мужики-то и не знают, что их string в дельфи (копия по записи) и std::string в крестах (копия по присваиванию), оказывается, доставляют им геморрой.
                    Ответить
                    • Кстати, а в делфи потокобезопасное COW?
                      Ответить
                      • Да хер знает, не смотрел, как-то потоками не интересовался.
                        Думаю да. Для интерфейсов вот счётчик ссылок сделан через какие-то InterlockedIncrement, видимо это для потоков фича, значит и для строк должно быть.
                        Ответить
                        • Если InterlockedIncrement, то, скорее всего все ок.
                          Ответить
                    • То-то С++ всё время течёт и падает :)
                      Ответить
                      • Не поэтому
                        Ответить
                        • Я не уверен, как там можно потокобезопасно сделать. Вот уверен течёт и падает на мультиядерных процессорах.
                          Ответить
                          • Это не из-за строк. Это из-за крестоблядской ауры.
                            Ответить
                          • Блин ну в Qt же сделали строки с потокобезопасным COW. Да и судя по словам Тараса даже в делфи они безопасны.
                            Ответить
                            • > с потокобезопасным COW
                              которые требуют атамарного инкремента счётчиков даже при копировании в рамках одного потока. Получается, что мы платим за многопоточность (если, конечно, не используются atomic типы современных процессоров, сомневаюсь, что в Delphi это есть) даже тогда, когда её не используем.
                              Ответить
                              • Да, к сожалению, компилятор не умеет использовать другие версии процедур для 1-поточной программы.
                                И вообще строки не то место, где надо экономить. Кому надо быстро - кастуют к пчару.
                                Ответить
                                • Топик наглядно демонстрирует нам, как всё тормозит, когда неправильно используешь строки.
                                  Ответить
                                  • Все эти многопоточные блокировки не меняют алгоритмическую сложность. А тема о ней.
                                    Ответить
                              • Там используется QAtomicInt - обертка над платформозависимым атомарным интом. Плата есть, не спорю. Но:
                                1) Во многих случаях строку передают по цепочке функций как const &.
                                2) В тех же случаях когда нужна именно копия объекта - атомарный инкремент обойдется дешевле выделения памяти и полного копирования (если строки не по 3 символа).

                                А в делфи, как кидал выше Тарас, используется InterlockedIncrement. Т.е. тоже atomic int.
                                Ответить
                                • > атомарный инкремент обойдется дешевле
                                  Если атомарный, то всё ок

                                  Оба подхода имеют право на жизнь, не спорю. Просто для того, чтобы делать строки иммутабельными, тоже есть веские причины.
                                  Ответить
                                  • Но эти причины конечно же не "Вот уверен течёт и падает на мультиядерных процессорах.".
                                    Ответить
                                • >Там используется QAtomicInt
                                  Атомарный инт только гарантирует, что число атомарно будет изменяться -- и всё. Дальше мы сравниваем число с нулём и джампим -- и тут -то оно уже не атомарно. Другой поток может "включиться" в середине сравнения с нулём и повредить объект. Короче, мороки много.
                                  Ответить
                                  • > Атомарный инт только гарантирует, что число атомарно будет изменяться -- и всё.
                                    Не только. Атомарные операции еще и атомарно возвращают новое или старое значение (смотря какая операция), в противном случае они были бы просто бесполезны.

                                    man атомарные операции
                                    man барьеры и memory ordering
                                    Ответить
                                    • Я прекрасно это знаю, я лишь говорил о том, что с точки зрения реализатора библиотеки нужно быть предельно внимательным, чтобы не допустить ошибок -- и если они допущены, то баг может проявиться только на определённой конфигурации, например. Плюс оверхед. Иммутабельные строки проще в реализации, тем более если например идёт портирование на другие платформы, где свои заморочки, другие компиляторы и проч.
                                      Мне кажется, конкатенация строк происходит значительно реже, чем копирование объекта в С++, поэтому мне не понятно зачем оверхедить на каждый шаг (а С++ копирует на каждом шагу по 500 раз) только потому, что строку теоретически могут конкатенировать где-то раз в год? Правильнее всё-таки сделать строку полностью иммутабельной -- универсально простое решение, с нулём оверхеда при передаче объекта; и вот уже построитель строк -- стрингбилдер -- пусть владеет shared-семантикой.
                                      Ответить
                                      • > Иммутабельные строки проще в реализации
                                        > с нулём оверхеда при передаче объекта
                                        А удалять эти самые иммутабельные строки в с++ кто будет? Пушкин?
                                        Ответить
                                        • Какой-нибудь shared_ptr
                                          Ответить
                                          • > Какой-нибудь shared_ptr
                                            С каким-нибудь атомарным счетчиком ссылок... От чего уходили к тому и вернулись ;)
                                            Ответить
                                            • Разница в том что COW forces атомарность и сопутствующий оверхед там где оно не нужно в 90% случаев; а shared_ptr позволяет самому решать.
                                              Ответить
                                        • Строки довольно редко shared'-ятся несколькими контекстами, поэтому удалять строки нужно явно. Если нужен автоматизм -- то пожалуйста shared_ptr.
                                          Ответить
                                          • Так все-таки, давайте определимся. Вы за иммутабельные строки, или же за std::string или тупо против COW и не имеете собственной позиции?

                                            > shared_ptr позволяет самому решать.
                                            Отключением поддержки тредов? Так и COW строки при отключении трединга перейдут на использование обычных операций вместо атомарных.

                                            Или, может быть, неиспользованием shared_ptr, там где очевидно, что строка будет в одном экземпляре - так и COW строки получат оверхед только в конструкторе и деструкторе, где крутятся new и delete, на фоне недетерминированного оверхеда которых, две атомарных операции будут незаметны.

                                            И да, COW строки, так же как и std::string и иммутабельные строки можно передавать по const & в вызываемые функции. Здесь оверхеда нет при любой реализации.

                                            А теперь, давайте, от оверхедов перейдем к семантике. Ведь, сейчас не 80е годы, и время программиста, рабающего над кодом стало намного дороже машинного времени. И здесь COW структуры данных представляют возможность реализовать нужный алгоритм, не заморачиваясь на такие мелочи как выделение и освобождение памяти, и где передать копию, а где шаред поинтер... А ведь код без этих преждевременных оптимизаций легче рефакторить и поддерживать. А это, имхо, почти всегда важнее оверхеда.

                                            P.S. Я ничего не имею против иммутабельных строк и std::string. Просто мне неприятно когда COW поливают говном и не видят в нем никаких плюсов.
                                            Ответить
                                            • >Просто мне неприятно когда COW поливают говном и не видят в нем никаких плюсов.
                                              Личное оскорбление. Тарас, будь моим секундантом.
                                              Ответить
                                          • > поэтому удалять строки нужно явно

                                            ты хуй
                                            Ответить
            • Вот в делфях строки мутабельные - там можно наращивать. В Java жа строки иммутабельны. Поэтому такая конкатенация приведет к перевыделению s.length+1 байта памяти под новую строку и копированию s.length байтов.
              Ответить
    • А нельзя это дело каким-нибудь jsp выводить?
      Ответить
    • Кстати, а почему экранируются символы, которые больше чем 255 а не 127?
      Ответить
      • Хз. Я вообще не понимаю, нафига этот код. Он вызывается на строку, полученную с помощью JsonElement.toString(). По идее, gson сам заэкранирует строки. Я так понимаю, человек боялся, что клиент не сможет понять кодировку. Но на всякий случай оставил, как есть. Только поменял конкатенацию на StringBuilder и выправил угрёбищный код по преобразованию к шестнадцатеричной системе.
        Ответить
        • > Только поменял конкатенацию на StringBuilder и выправил угрёбищный код по преобразованию к шестнадцатеричной системе.
          Имхо надо бы еще поменять 255 на 127. Маловероятно, что в юникодной строке попадутся символы с кодами 128-255, но я бы подстраховался.
          Ответить
          • Если подстраховываться, то по идее там вообще надо весь код выкинуть и по-человечески его переписать. Так что решил не заморачиваться и не подстраховываться
            Ответить
        • > gson сам заэкранирует строки
          Да вроде бы нет, не заэкранирует.
          Я пробовал как-то без указания кодировки (utf-8) gson.toString() в поток выводить, не работает.
          Ответить
          • gson заэкранирует всякие символы из диапазона 0x00..0x20. А всё, что выше 0x7F решается правильным выставлением заголовков HTTP
            Ответить

    Добавить комментарий