Пейте, дети, молоко! Пользуйтесь стандартной библиотекой! И будьте бдительны!

Забавную ситуацию вспомнил: как-то на ревью коллега стал меня убеждать в необходимости своих правок. Обосновывал тем, что "из теста вернули с комментарием — очень долго работает запрос, не можем дождаться ответа и обрубаем". Поэтому он решил изменить способ итерации, чтобы всё ускорилось

Краткая суть: был метод, возвращавший некие данные за период (12 месяцев с отмоткой назад, endDate = 12 месяцев назад, нижняя граница). Внутри себя - он итерировался по этим месяцам, дергал другой метод для получения помесячных наборов, и агрегировал их. Конструкция была такая:

LocalDate startDate = LocalDate.of(2022, 11, 1); LocalDate endDate = startDate.minusMonths(12); for (LocalDate date = endDate; date.isBefore(startDate); date = date.minusMonths(1))

Подобный метод-агрегатор был не один, и на ревью первого из них я коллеге посоветовал генерировать набор дат вызовом библиотечного АПИ datesUntil с шагом в месяц, и дальше сгенерированный набор использовать. Это дело благополучно забылось, даты продолжали создаваться внутри for и кочевали с копипастой, дожив до обсуждаемого момента

Я сильно засомневался, что в совершенно ненагруженном вызове, при итерации по 12 элементам — иной способ итерации даст какую-то заметную разницу. Сделал иллюстративные наброски в JShell (полноценные бенчмарки лень было, да и ни к чему) для сравнения разных способов с исходным. Типа таких:

LocalDate startDate = LocalDate.of(2022, 11, 1).plusMonths(1); LocalDate endDate = startDate.minusMonths(12); static List<LocalDate> datesList = endDate.datesUntil(startDate, Period.ofMonths(1)).collect(Collectors.toList()); static LocalDate[] datesArray = endDate.datesUntil(startDate, Period.ofMonths(1)).toArray(LocalDate[]::new); static TreeMap<Long, List<String>> results = new TreeMap<>(); long start, duration; start = System.currentTimeMillis(); for (int i = 0; i < datesList.size(); i++) { System.out.printf("\r %s", datesList.get(i)); } duration = System.currentTimeMillis() - start; results.computeIfAbsent(duration, k -> new ArrayList<>()).add("Dates list for - i"); start = System.currentTimeMillis(); for (LocalDate date = endDate; date.isBefore(startDate); date = date.plusMonths(1)) { System.out.printf("\r %s", date); } duration = System.currentTimeMillis() - start; results.computeIfAbsent(duration, k -> new ArrayList<>()).add("Dates in-place generated for each"); // ... results.forEach((key, value) -> value.forEach(testName -> System.out.println(testName + " => " + key)));

Внимательный читатель уже видит суть проблемы 🕵. Самый внимательный - уверен, увидел уже на третьем абзаце!

Результаты, ожидаемо, отличались крайне незначительно:

Dates list for - i => 692

Dates in-place generated for each => 704

Dates list for - each => 708

Dates array for - i => 708

Dates list forEach => 712

Dates list while + iterator => 1060

Разумного объяснения не было Стали закрадываться мысли - а не хлопнуть ли нам взяться ли нам за JMeter, профайлер, снять граф... Углубиться, так сказать, до дна.

Не пришлось - взглянул повнимательней и заметил разницу: date = date.minusMonths(1) vs date.plusMonths(1). Вот оно! Банальный бесконечный цикл с отмоткой назад от нижней границы периода.

Будь я бдительней - раньше бы заметил, что при копипасте цикла между методами - поменялось назначение границ периода, а итерация осталась старой. В одном методе работало ожидаемо, в другом - 💥.

Используй коллега библиотечный вызов - получал бы единообразно нужный период. И места для ошибки меньше, и проверять проще. И протестировано всё авторами. И оптимизацию бесплатную могут завезти.

Молоко, говорят - просто полезно для здоровья. Его даже за вредность дают. Но не всем 😿

1 комментарий