O tym jak wprowadzając clean code trzykrotnie zwiększyłem zużycie pamięci

detektywi.it 2 lat temu

Most papers in computer science describe how their author learned what someone else already knew.

Peter Landin

Ten post to krótka historia tego jak dobre chęci mogą doprowadzić do katastrofy, gdy zapomina się o wewnętrznych mechanizmach JVM oraz o tym jak, po raz kolejny, zadziałało podejście Kenta Becka – “Make it work, Make it right, Make it fast”.

Brzydki kod do refaktoryzacji

Niedawno trafiłem na dość nieczytelny kawałek kodu. Poniżej zamieszczam jego esencję

private Map<String, Map<String, Set<String>>> map = new HashMap<>(); void put(String a, String b, String c) { map.putIfAbsent(a, new HashMap<>()); map.get(a).putIfAbsent(b, new HashSet<>()); map.get(a).get(b).add(c); } boolean contains(String a, String b, String c) { return map.getOrDefault(a, new HashMap<>()) .getOrDefault(b, new HashSet<>()) .contains(c); }

Oczywiście w oryginalnym kodzie nie było tak wydzielonych metod, klasa miała kilkaset linii a wszystko było znacznie bardziej poplątane. Po kilkunastu krokach refaktoryzacyjnych, wymieniłem Map<String, Map<String, Set<String>>> na Set<Key> i uzyskałem coś w stylu

private Set<Key> set = new HashSet(); void put(String a, String b, String c) { set.add(new Key(a, b, c)); } boolean contains(String a, String b, String c) { return set.contains(new Key(a, b, c)); } record Key(String a, String b, String c) { }

Zmienne w przykładzie nazywam a, b, c, itd, aby nie wprowadzać zawiłości domeny produktu.

Zadowolony z efektów, wdrożyłem zmianę. Po kilkunastu minutach sprawdzilem metryki usługi, a na nich ogromny skok zużycia pamięci. Wdrożenie na szczęście było nie na środowisko produkcyjne.

Co poszło nie tak?

Okazało się, iż nowa struktura danych konsumuje kilkakrotnie więcej pamięci. Na tym nie koniec, nie jest to mała kolekcja, przechowuje ona miliony elementów. Kolekcja w oryginalnej wersji ważyła jakieś 400MB, a w „poprawionej”, około 1100MB.

Wzrost zużycia pamięci w tym przypadku wynika z mechanizmu tworzenia i przechowywania Stringów w JVM. Java ma szereg optymalizacji na stringach. W szczególności literały stringowe trafiają w pamięci do puli stringów i mogą być wielokrotnie używane, np.

String x = "Lorem ipsum"; String y = "Lorem " + "ipsum"; System.out.println(x.equals(y)); // prints true System.out.println(x == y); // prints true

Nie jest to prawda dla stringów, które nie są literałami, np.

int i = 0; String x = "Lorem ipsum" + i; String y = "Lorem ipsum" + i/2; System.out.println(x.equals(y)); // prints true System.out.println(x == y); // prints false

Można wymusić dodanie stringa do puli stringów dzięki metody intern(), ale to rozwiązanie ma swój zapaszek i w mojej ocenie może prowadzić do błędów.

Opisane zachowanie stringów sprawia, iż implementacja z Map w moim przypadku jest znacznie bardziej optymalna pamięciowo. W mapie trzymana jest referencja, co daje zauważalne zyski, gdy jest dużo wpisów o tym samym kluczu. Dodatkowo, w opisywanym przypadku, stringi były dość długie – liczyły po kilkadziesiąt znaków.

Głębsza analiza problemu

Aby lepiej zrozumieć co się dzieje w JVM, rozważmy następujący przykład

1 map.computeIfAbsent(new String("a"), x -> new HashSet<>()).put(new String("b1")); 2 map.computeIfAbsent(new String("a"), x -> new HashSet<>()).put(new String("b2"));

Po wykonaniu pierwszej linii kodu, utworzone zostaną następujące stringi:

"a" – literał, który może być sprzątnięty przez GC
"b1"– literał, który może być sprzątnięty przez GC
new String("a") – jako klucz, mapa trzyma referencję na ten obiektnew
String("b1") – wartość w set – set trzyma referencję na ten obiekt a mapa trzyma referencję na set.

Po wykonaniu drugiej linii kodu utworzone zostaną stringi:

"a" – literał, który może być sprzątnięty przez GC
"b2" – literał, który może być sprzątnięty przez GC
new String("a") – może być sprzątnięte przez GC ponieważ zostanie wywołany equals na stringu przy dodawaniu do mapy a taki klucz już istnieje
new String("b2") – wartość w set – set trzyma referencję na ten obiekt a mapa trzyma referencję na set.

W efekcie w pamięci trzymamy tylko trzy stringi – żadnych duplikatów.

Dla odmiany rozważmy wersję z set oraz obiektem agregującym.

1 set.add(new Key(new String("a"), new String("b1"))); 2 set.add(new Key(new String("a"), new String("b2")));

Po wykonaniu pierwszej linii, utworzone zostaną następujące stringi:

"a" – literał, który może być sprzątnięty przez GC
"b1" – literał, który może być sprzątnięty przez GC
new String("a") – wartość w obiekcie Key – Key trzyma referencję na ten obiekt a set trzyma referencje na Key
new String("b1") – wartość w obiekcie Key – Key trzyma referencję na ten obiekt a set trzyma referencje na Key

Po wywołaniu drugiej linii, utworzone zostaną stringi:

"a" – literał, który może być sprzątnięty przez GC
"b2" – literał, który może być sprzątnięty przez GC
new String("a") – wartość w obiekcie Key – Key trzyma referencję na ten obiekt a set trzyma referencje na Key
new String("b2") – wartość w obiekcie Key – Key trzyma referencję na ten obiekt a set trzyma referencje na Key

W efekcie w pamięci trzymamy cztery stringi które nie mogą zostać usunięte przez GC – instancja new String("a") jest przechowywana dwa razy.

Jak rozwiązałem problem

Ze względów wydajnościowych postanowiłem zostać przy mapie. Jednak całą brzydotę setu w mapy w mapie enkapsulując w osobnej klasy

static class MultiDeepMap<K1, K2, V> { private Map<K1, Map<K2, Set<V>>> map = new HashMap<>(); void put(K1 key1, K2 key2, V value) { map.computeIfAbsent(key1, it -> new HashMap<>()) .computeIfAbsent(key2, it -> new HashSet<>()) .add(value); } boolean contains(String key1, String key2, String value) { return map.getOrDefault(key1, new HashMap<>()) .getOrDefault(key2, new HashSet<>()) .contains(value); } }

Z takim API mamy czytelny sposób dodawania i sprawdzania czy jest coś dodane do mapy:

map.put("a", "b", "c") map.contains("a", "b", "c")

Aby zwiększyć czytelność kodu, można pozbyć się maniery typowania wszystkiego stringiem, żargonowa zwanej String Typeingu, opakowując Stringi w klasy/rekordy i wymienić

MultiDeepMap<String, String, String>

MultiDeepMap<A, B, C>

W moim przypadku wiązało się to ze wzrostem zużycia pamięci przez tą kolekcję o około 10%. Wszystkie pomiary wykonywane na Open JDK 17.0.1.