Rocky Runs Up The Stairs
Привет, Хабр. Вы, наверно, меня помните: я – Марко Кевац, системный программист в Badoo. Недавно я наткнулся на небольшой рассказ о том, как новичок сделал изменение в рантайме языка Go. Несмотря на то, что этот пост, наверное, довольно неожиданно встретить в хабраблоге Badoo, и многие могут сказать, что он банален и переполнен наивной радостью, я считаю, что такие истории демонстрируют, насколько сообщество Go доброжелательно и внимательно по отношению ко всем его участникам. Поэтому и перевел его.
А ещё в посте есть два интересных факта, связанных с внутренностями языка. Приятного чтения!
Я уже некоторое время пишу open-source-программы на Go. И вот только недавно у меня появилась возможность писать на Go и на работе. Я с радостью переключился и стал самым настоящим Go-разработчиком.
Всё было отлично, пока не случилась последняя конференция GopherCon, где проводили мастер-класс для тех, кто хочет делать вклад в развитие языка. И вот, видя, как все эти люди коммитят код в основной репозиторий, я тоже захотел что-то сделать. А буквально через пару дней Francesc Campoy записал прекрасное видео, в котором подробно рассказывает, как контрибьютить в Go.
Желание быть причастным охватило меня. Я понятия не имел, что я могу сделать, но решил скачать исходный код и скомпилировать его. Так и начался мой путь по дороге контрибьютора Go.
Я читал инструкцию для контрибьюторов и шаг за шагом выполнял её. Подписать CLA получилось не сразу, так как инструкция была немного корявой. Собственно, почему бы не указать на это и не предложить исправить её? Это может быть моим первым CL! Вдохновлённый, я создал тикет. Но оказалось, я наступил на стандартные грабли новичка. Проблема уже была решена в Tip, а я даже не догадался посмотреть.
Поскольку у меня уже всё было готово, я периодически просто гулял по стандартной библиотеке в поисках, что бы поправить. И поскольку я уже несколько месяцев программировал на Go на работе, я столкнулся с частями рантайма, которые постоянно всплывали во время профилирования. Одна из таких частей – пакет fmt. Я решил посмотреть на него внимательно и понять, можно ли что-то с этим сделать. Примерно через час я натолкнулся на кое-что интересное.
Функция fmt_sbx в файле fmt/format.go начинается следующим образом:
func (f *fmt) fmt_sbx(s string, b []byte, digits string) {
length := len(b)
if b == nil {
// No byte slice present. Assume string s should be encoded.
length = len(s)
}Было ясно, что len() вызывается два раза в случае, если b равно nil, хотя чаще всего достаточно одного. Но если передвинуть его в else, то len() вызовется всегда только один раз. Я решил отправить CL об этом, чтобы посмотреть, что скажут другие.
Буквально через пару минут Ian дал оценку +2, а затем Avelino – +1. Я не мог поверить в это!
Но потом что-то пошло не так. Dave поставил -1, и Martin – тоже. Он взял бинарный дамп кода и увидел, что между двумя вариантами нет никакой разницы. Компилятор был достаточно умным, чтобы сделать эту небольшую оптимизацию. Так что в итоге я оказался в минусе: else плохо сказывается на читабельности кода, а выигрыша в производительности никакого.
Этот CL пришлось забросить…
Но я узнал много нового. Например, о таких утилитах, как benchstat и benchcmp. Более того, теперь я был знаком со всем процессом и попробовать ещё раз мне ничего не стоило.
Вскоре я узнал, что простая конкатенация строк гораздо быстрее, чем fmt.Sprintf(). С этим знанием я пошёл искать «жертву», и это не заняло много времени. Я остановился на пакете archive/tar. Функция formatPAXRecord в файле archive/tar/strconv.go содержит следующий код:
size := len(k) + len(v) + padding
size += len(strconv.Itoa(size))
record := fmt.Sprintf("%d %s=%s\n", size, k, v)После того как я поменял последнюю строчку на record := fmt.Sprint(size) + " " + k + "=" + v + "\n", я увидел значительное ускорение:
name old time/op new time/op delta
FormatPAXRecord 683ns ± 2% 457ns ± 1% -33.05% (p=0.000 n=10+10)
name old alloc/op new alloc/op delta
FormatPAXRecord 112B ± 0% 64B ± 0% -42.86% (p=0.000 n=10+10)
name old allocs/op new allocs/op delta
FormatPAXRecord 8.00 ± 0% 6.00 ± 0% -25.00% (p=0.000 n=10+10)
Остальное, как говорится, уже история. На этот раз Joe отревьюил код, и после нескольких мелких исправлений его замёржили! Ура! Я внёс свой вклад в развитие Go. Я прошёл путь от средненького open-source-контрибьютора до контрибьютора в язык программирования Go.
Это далеко не конец. Я начинаю лучше понимать тонкости языка и буду продолжать создавать CL каждый раз, когда что-то найду. Десять чашек чая господам из Go-команды за то, что они так изящно и безустанно управляют разработкой такого сложного проекта.
P.S. Ну, и для справки:
- мой первый CL, который не приняли: https://go-review.googlesource.com/c/54952/;
- а это второй, который приняли: https://go-review.googlesource.com/c/55210/.
Комментарии (85)
laughman
29.08.2017 15:09+9length := len(b)
if b == nil {
это теперь нормально — сначала использовать аргумент, а потом проверять его на пустое значение?
myxo
29.08.2017 15:19-2len(nil) в go возвращает ноль. Так записано для читаемости.
Собственно это и хотел исправить автор вначале =)laughman
29.08.2017 16:02+16len(nil) в go возвращает ноль
и это сделал человек, утверждавший, что «errors are values»?
creker
29.08.2017 21:29-3Это сделали в языке, где нулевое значение максимально используется на полную катушку. Это важная часть языка. Поэтому nil массив это полезное значение, а не источник ошибок и постоянной необходимости проверок на nil. Конкретно код len(nil) не скомпилируется, а вот если в него передать переменную типа массив со значением nil, то действительно вернется 0. А еще при этом будет работать тот же cap() и append(). В итоге, и код чистый, и безопасностью никто не пожертвовал.
0xd34df00d
30.08.2017 02:54+1Конкретно код len(nil) не скомпилируется, а вот если в него передать переменную типа массив со значением nil, то действительно вернется 0.
Чтобы уточнить,
arr := nil len(arr)
соберётся, а
len(nil)
нет?
TheShock
30.08.2017 04:02+1Ни первое, ни второе не соберется — необходимо указать тип (use of untyped nil):
func main() { var arr []int = nil print(len(arr)) }
Второе — по аналогии можно собрать:
func main() { print(len([]int(nil))) }0xd34df00d
30.08.2017 04:54+5Я Go-то не знаю, это скорее псевдокод был. Спасибо за пояснение!
Но вообще это, конечно, грустно.
TheShock
29.08.2017 16:25+13len(nil) в go возвращает ноль. Так записано для читаемости.
Сложно назвать это читаемым. Говнокод как говнокод.
iamwizard
29.08.2017 23:49+3Я бы немного уточнил.
Магия тут не в len() — len всегда делает одно и то-же: берет поле Len из слайса, который представляет из себя структуру вида.
type Slice struct { Data uintptr Len int Cap int }
Магия тут в "касте" nil в слайс ( при сравнение или присвоении ). Сравнение слайса с nil — это сравнение со
Slice{0,0,0}что является значением по умолчанию при иницализации переменной слайса. В результате — только-что созданный слайс== nil
Зачем так было сделано? Как уже было написано — возможность работать со слайсом без дополнительных проверок. Это из того-же порядка что и отсутсвие необходимости инициализировать переменную типа
bool.
mayorovp
29.08.2017 16:28+17То есть считается, что код вида
length := len(b) if b == nil { // No byte slice present. Assume string s should be encoded. length = len(s) }
менее понятный, чем
if b == nil { length := len(s) } else { length := len(b) }
Кажется, я был прав когда не стал изучать этот язык...
Bibainet
29.08.2017 17:50+11Во втором примере вы на самом деле объявляете две разные переменные length, каждая в своей области {...}. Использовать их за пределами этой области не получится, там они не будут существовать. Такой код не скомпилится, потому что в Go нельзя объявлять локальную переменную и не использовать ее. Поэтому придется писать так:
var length int // Объявление if b == nil { length = len(s) } else { length = len(b) } // Используем length
Это значительно длинее первого варианта, хотя и понятнее.
mayorovp
29.08.2017 20:25+5Спасибо за исправление.
Это значительно длинее первого варианта, хотя и понятнее.
Ну так это же практически девиз языка go — "длиннее и понятнее". Но, видимо, даже у авторов стандартной библиотеки нету сил следовать ему полностью...
TheShock
29.08.2017 20:49+8Это значительно длинее первого варианта, хотя и понятнее.
В любом адекватном языке:
length := (b == nil) ? len(s) : len(b)
Или, даже, так:
length := len(b == nil ? s : b)
А иногда и так::
length := len(b || s)
И коротко, и понятно. Но в гоу все через жопу.creker
29.08.2017 21:31-10И все ваши примеры сложнее и менее читаемы, чем оригинал.
TheShock
29.08.2017 22:09+7Вы про тот оригинал, где берется длина nil, она оказывается равной 0 вместо ошибки, присваивается переменной, а потом внезапно передумывается и присваивается длина чего-то другого, да? Читаемее? Проще? Может еще и логичнее? Не смешите моего кота!
В то время какlen(b || s)— читается очень просто: дай длину b (если существует) или sFalstaff
29.08.2017 22:54Можно я с вами поспорю? :) Я вот, не зная этой конструкции, прочитал "дай длину b или s… ну чего-нибудь из этих двоих, всё равно чего". Нет, серьёзно, вот этого "если существует" в этой записи нигде нет, руку на сердце положа. Это такая же особенность языка как length(nil) в Go, которую тоже надо знать, просто она для вас привычнее. Гхм, ну и length(nil) тоже в принципе логику некую имеет — чему равна длина чего-то несуществующего? Ничему, нулю. (Если что, я не пишу ни на Go, ни на JS, просто погулять вышел.)
TheShock
30.08.2017 00:28+3А если это специальный оператор, у которого только такая задача?
Дело в том, что я знаю такую особенность Гоу (я на нем писал последнее время) и мне она кажется крайне нелогичной — это костыль в спецификации, потому что нету удобного инструмента.
чему равна длина чего-то несуществующего?
Ошибке (потому что в Гоу обычно возвращается ошибка в таких нелогичных случаях).
Но проблема ведь не только в len(nil), но и в том, что сперва ты присваиваешь неправильное значение, а потом проверяешь условия и присваиваешь правильное! Бред ведь.
Еще раз, я писал на Гоу, мне привычный этот синтаксис и я все-равно считаю его ущербным.Falstaff
30.08.2017 01:13Возможно, в каком-то языке это такой специальный оператор, у которого именно такая задача. Возможно, в другом языке это вообще бы значило "дай длину true если b — true или a — true, или длину false если оба false" — и, нет, я не могу ответить, какой физический смысл у len(true). :) Хорошо, для тех, кто пишет на C#, этот код читается просто. Откуда следует, что он универсально простой для всех? Для его понимания как минимум надо знать особенность конкретного языка, вот я про что.
Про длину несуществующего — я говорю не с точки зрения соответствия вообще конвенциям языка (я так глубоко в Го не лез, не моя епархия), а просто с точки зрения логики, не привязанной к какому-то языку. У ничего и длина ничего.
Да, если что — я ни разу не защищаю код из статьи, присвоение а потом проверка (для меня, но я не пишу на Го) вызывает содрогание. Но вот для примера — если бы там дальше стоял цикл (гипотетически, в какой-нибудь другой функции) и он бы при b = nil корректно отрабатывал 0 раз (поскольку len(nil) = 0) и функция возвращала бы пустую коллекцию, то это могло бы читаться нормально. По Мартину прямо. Так что я не думаю, что len(nil) = 0 это так уж нелогично. Код из статьи — ну, да. Но если я скажу "но логичнее было бы возвращать ошибку" или "логичнее было бы выбрасывать исключение", то я просто буду примерять ко всему набор вывихов, которые у меня в мозгу для другого языка, и не факт, что они на самом деле логичнее, просто я к ним привык.
TheShock
30.08.2017 04:22+5Так в том же и проблема — это кривой код как с точки зрения Гоу, так и с точки зрения computer science в целом.
И если второе очевидно, то первое объясню — обычно (та почти всегда, наверное) Гоу не позволяет делать таких вещей и выбрасывает ошибку. Та он выбрасывает ошибку даже когда ее можно не выбрасывать, просто чтобы выбросить.
Временно закомментил код, но не закомментил импорт — ошибка «imported and not used». Блин, я же дебажу, я его потом разкоменчу! Но нет, го--- оправдывает свое название и вместо того, чтобы написать ворнинг — падает в панику
package main import "fmt" func main() { // fmt.Println("Hello, playground") }
Разрабатываешь, пишешь логику, заранее объявил переменные, чтобы потом заюзать, решил проверить, как оно работает часть, которую написал, запускаешь. Фиг там — паника «declared and not used».
package main func main() { var arr []int = nil }
Ну то есть этот код никак не влияет на производительность, компилятор знает, что он лишний (все-равно провел анализ) и может быть спокойно выброшен, но нет ведь — ошибка
Имеем две структуру, которые циклически ссылаются одна на другую. Все работает корректно. Рефакторим, разносим их по разным неймспейсам. Все упало, внезапно гоу осознал циклическую зависимость! И я понимаю, что иногда это свидетельствует о неудачном дизайне, но я хочу сам решать. Тем более, самое популярное решение — не изменить архитектуру, а забить на рефакторинг и все свалить в однусвалкунеймспейс — там циклические зависимости можно.
Я уж молчу о тошнотворном повторении как мантрыif err != nil(это настолько въедается, что ты просто раз за разом копипастишь, как в анекдоте про студента и гвозди). Раз гвоздь, два гвоздь, но у тебя уже их в заднице столько, что ты привык:
a, err := doA() if err != nil { return nil, err } b, err := doB(a) if err != nil { return nil, err } c, err := doC(b) if err != nil { return nil, err }
Видите, как Go относится к таким ошибкам? Это и есть Go-way. И гоу-штунды кричат: «это не так плохо», «это даже клево», ведь это очевидно, ты должен контролировать свой код, ты должен писать свою архитектуру, ошибка должна быть явной, рекурсивная зависимость — плохой код, срочно рефакторите.
Но последовательность им не знакома, потому, на говнокод они кричат: «зато так покороче», ну и что, что он nil воспринимает как пустой массив, зато так удобно, нет ничего страшного, что я сначала присваиваю переменной неправильное значения, вызывая функцию len, когда ее не надо вызывать!
Зато неиспользуемые import'ы использовать нельзя — ведь Гоу следит за чистотой кода, видите ли.Falstaff
30.08.2017 05:26… это кривой код как с точки зрения Гоу, так и с точки зрения computer science в целом.
Это про len(nil), или именно про код из статьи? Если про код — да, я в принципе не спорю, можно было создать переменную без лишнего вызова len() с потенциальным мусором в аргументе. Я только про то, что контракт len(nil) -> 0 в принципе не криминал и может пригодиться в какой-нибудь другой ситуации.
С неиспользуемыми переменными и импортами да, тоже перегиб. Согласен — иногда хочется что-то набросать и проверить, или выкусить фрагмент на время. (Вроде goimports хотя бы об импортах позволяет забыть, но это если IDE.)
А что видится альтернативой цепочкам if-ов с проверками err? Исключения? Вообще — я отчасти понимаю, почему их нет. Язык бедный, он просто намеренно бедный — ему вроде бы как раз в заслугу ставят то, что команда вчерашних студентов может через неделю выдавать более или менее удобоваримый код, а не сидеть с опухшими головами и отвлекать сеньоров вопросами. Исключения всё-таки предполагают осознание того, что у нас в коде куча неявных точек возврата, кривая обучения сразу круче становится. (Если что, я за выразительные языки. Просто философски отношусь к тому, что рынок хочет толпу годных в дело стажёров.)
0xd34df00d
30.08.2017 07:40+2А что видится альтернативой цепочкам if-ов с проверками err? Исключения?
Монадическая обработка ошибок на манер Either.
SirEdvin
30.08.2017 09:09+3Язык бедный, он просто намеренно бедный — ему вроде бы как раз в заслугу ставят то, что команда вчерашних студентов может через неделю выдавать более или менее удобоваримый код, а не сидеть с опухшими головами и отвлекать сеньоров вопросами.
Она в любом случае не может. Какие такие задачи они будут решать? Люди не спотыкаются, о синтаксис языка, если это только не какой-то haskell, люди страдают от сложности библиотек и фреймоворков, но тут то go даже хуже, чем другие языки, потому что выразительные конструкции заменяются на структуры и кучи шаблонного кода.
Falstaff
30.08.2017 19:40Я не уверен, что это корректный аргумент. Вроде есть отзывы о том, что новички очень быстро становится продуктивным на Го (на бэкенде во всяком случае). И с другой стороны, выразительные конструкции могут, наоборот, усложнить понимание фреймворков и библиотек. Регулярно слышу, как народ стонет по поводу библиотек на Scala, которые без автора сопровождать невозможно.
0xd34df00d
30.08.2017 19:48Люди не спотыкаются, о синтаксис языка, если это только не какой-то haskell
В хаскеле, кстати, предельно простой синтаксис. Не лисп, конечно, но всё же.
0xd34df00d
30.08.2017 07:39+2Я уж молчу о тошнотворном повторении как мантры if err != nil
Я в каком-то соседнем треде уже предлагал, но предложу ещё раз: давайте замутим стартап по продаже USB-донглов с кнопкой, которая по нажатию выдаёт
if err != nil { return nil, err }.
laughman
30.08.2017 10:35+1a, err := doA() if err != nil { return nil, err } b, err := doB(a) if err != nil { return nil, err } c, err := doC(b) if err != nil { return nil, err }
интересно, а как при этом принято логировать такие ошибки?laughman
30.08.2017 10:41+1т.е. строчек по обработке ошибок должно бы быть больше, и смысловые строчки реально станут настолько редкими, что будет трудно со стороны отследить корректность алгоритма? или есть какой-то способ обойти проблему?
splav_asv
29.08.2017 22:41+1А так:
или так:let length = if let Some(s) = s {s.len()} else {d.len()};
?let length = s.unwrap_or(d).len();0xd34df00d
30.08.2017 02:58+2len = length $ fromMaybe d ssplav_asv
30.08.2017 09:54Да, такой вариант мне тоже всегда нравился. Но в силу вынужденных компромиссов приходится довольствоваться теми, что выше.
Alexeyco
29.08.2017 18:08+1Вы можете составить список языков программирования, фреймворков и фильмов, которые считаете негодными? Я чувствую, там много хорошего.
mayorovp
29.08.2017 20:23-4Языки программирования: Ada, Go и Papyrus Script
Фреймворки: Angular, redux*
*да, я знаю что это библиотека, а не фреймворк. Но к библиотеке у меня претензий как раз и нет.
rraderio
29.08.2017 17:28-1конкатенация строк гораздо быстрее, чем fmt.Sprintf().
После того как я поменял последнюю строчку на record := fmt.Sprint(size) + " " + k + "=" + v + "\n", я увидел значительное ускорение
Почему компилятор сам этого не делает?
powerman
29.08.2017 20:34+1Почему компилятор не знает всё про внутреннюю логику и побочные эффекты какой-то функции какого-то пакета и не заменяет её вызов на вызов другой функции? Ну, что Вам сказать… придётся признать, что разработчики компилятора невероятно ленивы и толком не умеют писать компиляторы.
А если серьёзно, то, как уже упоминали выше, такого рода оптимизации во-первых зачастую преждевременные, во-вторых ухудшают читабельность, в-третьих в следующих версиях языка и/или пакета fmt более медленный вариант сейчас может оказаться более быстрым, в-четвёртых если уж очень хочется, то рекомендации по такого рода оптимизациям должны давать внешние утилиты вроде статических анализаторов кода (линтеров) или IDE.
0xd34df00d
30.08.2017 03:01+1Почему компилятор не знает всё про внутреннюю логику и побочные эффекты какой-то функции какого-то пакета и не заменяет её вызов на вызов другой функции?
Да не, почему. Авторы функции, предполагая, что sprintf используется часто, и вложиться в производительность имеет смысл, могли бы написать правила по компил-тайм-парсингу строки формата во что надо, получив требуемый результат. От языка требуется только предоставить доступ к AST, все такие и подобные случаи покроются с лихвой.
А, в Go так нельзя? Ну сорян.
Falstaff
30.08.2017 04:01-1А часто ли? Я вот просто не уверен, что стоит огород городить для очень частного случая. Если у горстки человек sprintf() внезапно окажется одной из самых долгих вещей на горячем пути выполнения — ну, наверное они этот путь всё равно будут профилировать, и сделают то же, что сделал автор оригинальной статьи. А просто так, чтобы было… не знаю. Да даже подвиг автора взять — скажем, взять большущий архив, миллион файлов, по… скажем, пять записей в расширенном pax-заголовке для каждого из файлов. Двести наносекунд экономии на запись, и получится экономия аж в одну секунду на создание архива. Кошкины слёзы, а читаемость кода ухудшилась.
aml
30.08.2017 08:04-2Как бы мне этого ни хотелось, бюджет у go-team не бесконечный — все фичи не реализуешь. Я думаю, если напишете такой оптимизатор, вам только спасибо скажут.
0xd34df00d
30.08.2017 08:13Это не оптимизатор, это некоторое свойство языка, позволяющее программе на этом языке, опять же, иметь доступ к AST и всяко им манипулировать на этапе компиляции.
neolink
30.08.2017 13:12а есть примеры кода как это выглядит в реальной жизни (я так понимаю речь про haskel)?
0xd34df00d
30.08.2017 20:14+2Конкретно примеры со sprintf, чтобы с модификаторами форматирования и всем таким, мне неизвестны. Однако, вполне возможно написать библиотеку (заняться, что ли?), которая бы использовалась примерно так:
import MyPrintf fmtSomeArgs :: Int -> String -> Double -> String fmtSomeArgs num str anotherNum = [p|%d %s %4.2f|] num str anotherNum
с парсингом строки формата на этапе компиляции, проверкой типов и прочими ништяками.
Если упрощать, вот эта вот конструкция
[ident|...|]говорит компилятору, что надо передать функцииident(которая является специальной хреновиной — квазиквотатором) содержимое между скобок и практически синтаксически вставить в точку вызова результат этой функции.
Как пример того, что уже есть — есть куча библиотек для интерполяции, вроде такого:
import Data.String.Interpolate data Person = { name :: String, age :: Int, income :: Double } fmtPerson Person { .. } = [i|This is #{name}, #{age} years old, their income is #{income} and they #{if income > 100000 then "are" else "are not"} rich|]
Аналогично делаются raw string literals. Есть библиотека для вызова шелла и сторонних процессов:
import System.Command.QQ copySomeStuff = do exitCode <- [sh|scp #{remoteHost}:#{remotePath} #{localPath}|] guard $ exitCode == ExitSuccess sha1 <- [sh|sha1sum #{localPath}|] putStrLn sha1
Тут, кстати, и прелести системы типов видно —
shумеет определять по контексту, хотят от него код возврата, stdout или что-то ещё.
Есть библиотека для порождения типов dbus-функций:
import DBus.QuasiQuoter [dbus| s -> s a{uv} |]
опишет соответствующий тип хаскелевской функции.
Можно этой ерундой парсить XML в компил-тайме, можно генерировать XML, можно вообще много чего делать. Самая прелесть — ничего этого не надо держать в ядре языка, всё это можно делать библиотеками.
darth_dolphi
29.08.2017 23:22а буффер быстрее конкатенации…
neolink
30.08.2017 13:03здесь в общем случае нет, так как компилятор сам посчитает длинну и выделит сразу память под конечную строку
darth_dolphi
31.08.2017 15:50это точно? что то я сомневаюсь, компилятор ничего не знает о переменных что будут в рантайме, как он может сразу под конечную строку выделить память?
sand14
29.08.2017 18:01+4Лучше было бы создать функцию вычисления длины (пишу в псевдокоде):
int getLength(string b, s) { if (b != null) return len(b); return len(s); // return b != null ? len(b) : len(s); }
А затем в основной функции вызвать ее так:
length = getLength(b, s);
Что получаем:
Код основной функции становится чище — в нем показывается, что нужно делать, а не как делать.
Во вспомогательной функции последовательно проверяем "особые" условия (которые теоретически могут добавляться), каждый раз return-имся, в конце возвращаем основное значение.
Да, избавляемся от действительно не очень красивого "else" в коде основной функции.
И самое главное — избавляемся от того, что код, приведенный в начальном варианте:
length := len(b) if b == nil { // No byte slice present. Assume string s should be encoded. length = len(s) }
не просто не оптимален, но и не отражает сути того, что нужно получить в результате работы этого кода, а значит, потенциально может уже содержать ошибки, не является легко сопровождаемым и масштабируемым.
- А на возражение, что вызов функции — накладные расходы, ответить очень просто: а вот пусть в этом случае компилятор показывает чудеса оптимизации и инлайнит вызов этой функции.
Если вспомогательная функция находится в том же модуле/пакете/сборке, и не видна наружу (internal), то ее вполне можно заинлайнить по умолчанию; хотя можно и помочь компилятору, указав соответствующий атрибут, если таковой есть в языке/платформе.
Alexeyco
29.08.2017 18:13-7С вашим подходом не получится хейтить go абсолютно без каких-либо на то оснований. Там выше уже много гигантов программизма, которым достаточно довода «а мне прост не нра», чтобы язык назвать плохим.
SirEdvin
30.08.2017 09:11+2Интересно, а какие доводы в пользу неудачного синтаксиса вы ожидаете? Исследований, которые скажут, что да, вот такой синтаксис приводит к 100к ошибкам в месяц?
Alexeyco
30.08.2017 11:40+1Объективно было бы сравнить с другими языками, противопоставить одной точке зрения другую. Но как можно ожидать объективности от людей, которые как ошалелые минусуют комментарий, в котором констатируется факт: областей видимости в привычном виде в go нет? И побежали минус-минус-минус. Часто слышу от молодняка слово «совок». Ну так я в данный момент в нем. Вот он — совок-то.
SirEdvin
30.08.2017 11:57+2В привычном — это в каком? У каждого языка эта "привычность" довольно своя. Вот у python там один ад, в javа вот там все понятно.
Alexeyco
30.08.2017 12:12-1Хорошо, в явном. В таком, где нужно было бы писать private, public etc. В go области видимости — часть синтаксиса. Иногда это бесит. Но если поумерить свое эго, то нормально.
SirEdvin
30.08.2017 12:24private, public ect — это модификаторы доступа. Они не имеют отношения к области видимости.
Ну, так в языках, с которыми я знаком (Java, C#), где-то по другому?
Alexeyco
29.08.2017 18:17-5В go, кстати, нет областей видимости, которые надо явно указывать. Если функция или метод структуры начинается со строчной буквы — она не видна извне. А если с заглавной — то видна.
creker
29.08.2017 21:40-4Вот когда этих условий станет слишком много, тогда и стоит выделять в функцию. У вас видно больше какое-то маниакальное стремление выделить ерунду в отдельную функцию, тем самым ухудшив читаемость. И не только потому, что эти несчастные несколько строк надо искать по коду. Как раз название getLength абсолютно не отражает сути того, что же сделает функция, да еще и написали вы ее неправильно — изначальный вариант содержит аргументы b и s разных типов. Какую длину, чего она вернет — сумму длин? Одной из двух? Если одну из двух, то по какому условию?
sand14
30.08.2017 00:13+3Представляется, что в этом в т.ч. и состоит искусство разработчика — исходя из опыта и знания предметной области уметь загодя определять, будут ли разрастаться в будущем "особые" условия, потребующие отдельной функции.
И если "да" — то эту функцию лучше сделать сразу, т.к. практика показывает, что "потом", особенно при совместном владении кодом, никто не делает декомпозицию, необходимость которой появилась. В итоге разрастаются простыни кода.
Также верно, что в случае, когда функция имеет значение только в контексте какой-либо другой функции, то ее имя требует осмысления (т.к. тот же getLength представляется именем функции общего назначения, а не контекстно-зависимой).
На мой взгляд, хорошим решением здесь является механизм локальных функций или лямбд (насколько знаю, в Golang нет локальных функций, но есть лямбды).
Но вы почему-то написали несколько придирок не по существу к моим тезисам, и проигнорировали основное — что речь шла о возможных способах замены этого кода:
length := len(b) if b == nil { // No byte slice present. Assume string s should be encoded. length = len(s) }
Это же никуда не годится.
Во-первых, нужно помнить, что первая строчка не упадет с исключением.
Точнее, в случае Go — что функция len имеет декларацию "func len(v Type) int", а не "func len(v Type) (int, error)".
Но этот код может читать и неподготовленный человек.
Во-вторых, это же хак — рассчитывать на то, что компилятор транслирует этот исходный код в бинарный так, как будто он написан через else.
Нечитаемо, и кроме того, поведение компилятора ведь может и измениться.Alexeyco
30.08.2017 11:47-3Сплошные «может быть» и «наверное». Мы вынуждены поверить, что написать так — это правильно просто потому, что «может быть». А «может быть» это ваше существует только потому, что аргументов нет. Есть аргумент — код стал короче. В данном конкретном случае короче он стал из-за упрощения конструкции. Но мы, конечно, должны исходить из того, что компилятор «может быть» такой код воспримет хуже. И конечно, варианта, что такой код он воспримет лучше, быть не может. Все правильно.
В 1933 году Петр Борисович Ганнушкин ввел термин "салонное слабоумие". Символично, но я готов дополнить его теорию. Те, кто не хочет признавать истину авторитетов должны быть такими вот салонно слабоумными изгнаны.SirEdvin
30.08.2017 11:59+3В 1933 году Петр Борисович Ганнушкин ввел термин "салонное слабоумие". Символично, но я готов дополнить его теорию. Те, кто не хочет признавать истину авторитетов должны быть такими вот салонно слабоумными изгнаны.
Звучит иронично, потому что 90% golang построено на авторитетный решениях, с которыми далеко не все согласны.
Alexeyco
30.08.2017 12:16-3Правильно! И они что делают? Аргументируют. Может быть, я ошибаюсь, но как-то не припомню, чтобы те или иные решения были выкачены просто потому. Обычно аргументация присутствует. Либо говорят «мы считаем, что так и так — это правильно». Ок, я услышал — это точка зрения.
Пример — отсутствие исключений. Долгий был срач на эту тему, но аргументация от разработчиков была. Разве нет? Если бы они заявили «исключений не будет потому, что они не нужны» без объяснения, почему не нужны или с неубедительной аргументацией, я бы так же точно ополчился на них.
Но вообще-то надо отвечать за себя. Причем тут ваш комментарий?SirEdvin
30.08.2017 12:27+2Аргументируют.
Или делают вид, что аргументируют. С одной стороны "у нас логичный и строгий язык", а с другой "len(nil) == 0". С одной стороны "исключения плохо, потому что везде приходится фигашить проверки", а по факту стало еще хуже, так как вместо типизированных исключений мы получаем переменную err, которую так же надо обрабатывать, но она менее прозрачная. Шаблонные типы не нужны, потому что это "усложняет язык", а вот кодогенерация — нет.
Alexeyco
30.08.2017 12:31-2С третьей стороны, когда анонсировали 2.0 (или рассуждали о том, каким он будет), я своими глазами помню как они написали что-то типа «да, мы говорили, что дженерики — гумно, но если надо, то давайте сразу подумаем, чего не хватает, чтобы это заранее заложить».
Alexeyco
30.08.2017 12:22-2Можно вас и вообще всех попросить слить мне карму окончательно, чтобы каждый раз, когда у меня возникнет желание что-то прокомментировать, я сразу же это видел и бросал это дело? Спасибо.
SirEdvin
30.08.2017 12:24У меня вообще не хватает рейтинга, что бы карму сливать :)
Alexeyco
30.08.2017 12:27-4Прошу прощения. Я тогда дополню, чтобы лучше сливалось. Хуесосы. Вы все. Администрация — тоже. Я так не думаю, но мне поднадоело. Я знаю, что если мне не слить карму, я рано или поздно приду к мысли «да ладно, напишу комментик».
sand14
30.08.2017 00:28+1По-моему, ваш первый пул-реквест нужно было принять (почему — написал выше), а вот второй как раз отклонить:
- Конкатенация строк вместо форматирования очень сильно снижает читаемость кода.
- Что касается снижения производительности в случае использования форматирования строк, то представляется, что функция, которая внутри себя форматирует строки, явно не предназначена для вызова в цикле из миллиона итераций.
А в случае однократного (или, пусть несколько десятков) вызова — как раз тот случай, когда можно пожертвовать производительностью даже в несколько раз ради читаемости.
Кроме того, если эта функция форматирует строки, вряд ли она предназначена для вызова рядом функциями, быстро вычисляющими какую-нибудь математику. Скорее всего, эта функция вызывает в контексте каких-то еще более медленных функций.
bat
30.08.2017 07:55size := len(k) + len(v) + padding size += len(strconv.Itoa(size)) record := fmt.Sprintf("%d %s=%s\n", size, k, v)
а что будет, если len(k) + len(v) + padding = 99 ??neolink
30.08.2017 12:58и что же будет?
bat
30.08.2017 13:07будет баг нет?
как я понял size так же учитывает и свою собственную длину в строковом представлении
bat
30.08.2017 13:11+2точно, в глянул в оригинале, там повторное кодирование строки, если длина не та
это ж пипец
// formatPAXRecord formats a single PAX record, prefixing it with the // appropriate length. func formatPAXRecord(k, v string) string { const padding = 3 // Extra padding for ' ', '=', and '\n' size := len(k) + len(v) + padding size += len(strconv.Itoa(size)) record := fmt.Sprintf("%d %s=%s\n", size, k, v) // Final adjustment if adding size field increased the record size. if len(record) != size { size = len(record) record = fmt.Sprintf("%d %s=%s\n", size, k, v) } return record }
ps
формат PAX record весьма неудачен, имха
pps
CL автора такие не принятneolink
30.08.2017 13:42+1ну его поправили чутка и приняли: github.com/golang/go/commit/23cd87eb0a2d49a3208824feaf34d8b852da422f
ну
func formatPAXRecord(k, v string) (string, error) { const padding = 3 // Extra padding for ' ', '=', and '\n' size := len(k) + len(v) + padding sizeLen := len(strconv.Itoa(size)) size += sizeLen sizeStr := strconv.Itoa(size) if len(sizeStr) != sizeLen { sizeStr = strconv.Itoa(size + len(sizeStr) - sizeLen) } record := sizeStr + " " + k + "=" + v + "\n" return record, nil }
не знаю насколько читаем этот вариант, но для начального кода:
BenchmarkFib10-8 10000000 163 ns/op 32 B/op 2 allocs/op
для этого
BenchmarkFib10-8 20000000 94.9 ns/op 16 B/op 1 allocs/op
тоже чтоли пул риквест сделатьbat
30.08.2017 15:17хотел придраться к тому что strconv.Itoa(size) вызывается дважды, но бенч показывает одну алокацию ))
neolink
30.08.2017 17:00ну size то меняется между ними поэтому тут одной никак
по поводу аллока — это то что выделяется в куче, то что выделяется на стеке там не показывается (ибо быстро и так легко не отследишь)
кстати это на 1.9 были результаты ради интереса проверил на 1.8:
BenchmarkFib10-8 5000000 247 ns/op 32 B/op 5 allocs/op
BenchmarkFib10-8 10000000 181 ns/op 16 B/op 4 allocs/op
получается неплохо так доработали компилятор в 1.9 — как миниму он сумел все вызовы strconv.Itoa на стеке разместитьbat
30.08.2017 18:53была мысль что заинлайнил вызовы strconv.Itoa и разместил результат на стеке
отличный результат на 1.9 надо переходить
sand14
30.08.2017 19:24там целом повторяется строка
strconv.Itoa(size) + " " + k + "=" + v + "\n"
как и исходная
fmt.Sprintf("%d %s=%s\n", size, k, v)
Lenz007
31.08.2017 12:19+3Странная логика у автора, вместо того чтобы попытаться оптимизировать работу библиотечной функции fmt.Sprintf() он заменяет ее на черти что, и называет это оптимизацией.
playnet
>После того как я поменял последнюю строчку на record := fmt.Sprint(size) + " " + k + "=" + v + "\n", я увидел значительное ускорение
… И существенно менее читаемый код. В модуль не смотрел, но если это работает в цикле и вызывается часто — быстродействие важнее, но если 1-2 раза на упаковку — это плохой, вредный код. Ну или хотя бы надо оставить старый код как комментарий, для понимания.
0xd34df00d
Не надо. Потому что когда потом новый код поменяют, а старый поменять забудут (или поменяют неправильно, он же закомментированный, кто его тестировать будет?), будет весело.