rmoretto
Help to simplify/optimize a function
Hello!
So I currently have the following problem: I want to take a map and “sanitize” all the integers fields. Basically verifying if that field value is really an int and replace the parsed value in the map, furthermore, if the verified value is not an valid int it should save nil to the field, for example:
Input Map
%{
int_a: "1",
int_b: "not_a_int!"
%}
Output Map:
%{
int_a: 1,
int_b: nil
%}
On the system that I am currently working I know that all the integers fields in the map will start with the int_ prefix (important to say that I don’t have full access to the struct of the map, so I can’t write a function that uses a specific field) so I wrote the following code:
def parse_map_int_fields(map) do
ints_map =
map
# Filter all the Integers field
|> Enum.filter(fn {k, _v} -> Atom.to_string(k) |> String.starts_with?("int_") end)
# Remove all fields that are nil or ints
|> Enum.filter(fn {_k, v} -> !is_integer(v) and !is_nil(v) end)
# Parse the values to int, returning nil if invalid
|> Enum.map(fn {key, val} ->
case Integer.parse(val) do
:error ->
{key, nil}
{parsed, _} ->
{key, parsed}
end
end)
|> Map.new()
# Merge the original map with the "sanitized" map
Map.merge(map, ints_map)
end
Another point is that this code will be ran in a lot of maps (5 to 10 millions maps) with on average 15 fields per map, so I have two questions:
- Most important, can this code be simplified? From my perspective it look a little bit convoluted, I would appreciate any tips!
- How one would optimize this function? I have wrote a simple benchmark using Benchee that gave me the following results:
Operating System: Linux
CPU Information: AMD Ryzen 7 2700X Eight-Core Processor
Number of Available Cores: 16
Available memory: 15.63 GB
Elixir 1.11.2
Erlang 23.2.3
Benchmark suite executing with the following configuration:
warmup: 2 s
time: 5 s
memory time: 0 ns
parallel: 1
inputs: none specified
Estimated total run time: 7 s
Benchmarking parse_map_int_fields...
Name ips average deviation median 99th %
parse_map_int_fields 857.59 K 1.17 μs ±2149.16% 1.01 μs 2.12 μs
Code used for the benchmark:
defmodule Sanitizer do
def parse_map_int_fields(map) do
ints_map =
map
|> Enum.filter(fn {k, _v} -> Atom.to_string(k) |> String.starts_with?("int_") end)
|> Enum.filter(fn {_k, v} -> !is_integer(v) and !is_nil(v) end)
|> Enum.map(fn {key, val} ->
case Integer.parse(val) do
:error ->
{key, nil}
{parsed, _} ->
{key, parsed}
end
end)
|> Map.new()
Map.merge(map, ints_map)
end
end
test_map = %{
int_a: "1",
int_b: "not_a_int!",
str_a: "This is a string field",
str_b: "Another string field",
fl_a: 0.0
}
Benchee.run(%{
"parse_map_int_fields" => fn -> Sanitizer.parse_map_int_fields(test_map) end
})
Which isn’t all that bad for my use case, but in the interest of learning I would like to know if something could be done different.
Thanks to all! 
Most Liked
gregvaughn
I haven’t benchmarked, but you could do something with a for comprehension
iex(103)> map = %{
...(103)> int_a: "1",
...(103)> int_b: "not_a_int!",
...(103)> other: "leave me alone"
...(103)> }
%{int_a: "1", int_b: "not_a_int!", other: "leave me alone"}
iex(104)> for {k, v} <- map, match?("int" <> _, Atom.to_string(k)), into: map do
...(104)> case Integer.parse(v) do
...(104)> {int, _} -> {k, int}
...(104)> _other -> {k, nil}
...(104)> end
...(104)> end
%{int_a: 1, int_b: nil, other: "leave me alone"}
rmoretto
Thanks people for all the responses!
Indeed a lot of way to approach the problem, I will probably use the @amnu3387 response just because is a little bit faster, but as Eric said, is just a matter of taste, and all other options would be completely fine.
Thanks again for the help!
The benchmark results were:
Name ips average deviation median 99th %
mnussbaumer_response 927.74 K 1.08 μs ±2239.93% 0.96 μs 1.98 μs
kokolegorille_response 867.51 K 1.15 μs ±2327.39% 1.00 μs 2.14 μs
gregvaughn_response 766.18 K 1.31 μs ±2168.73% 1.15 μs 2.31 μs
ericgray_response 730.43 K 1.37 μs ±1314.86% 1.26 μs 2.48 μs
chungwong_response 655.58 K 1.53 μs ±1555.15% 1.36 μs 2.78 μs
parse_map_int_fields 567.61 K 1.76 μs ±1491.38% 1.57 μs 3.18 μs
Comparison:
mnussbaumer_response 927.74 K
kokolegorille_response 867.51 K - 1.07x slower +0.0748 μs
gregvaughn_response 766.18 K - 1.21x slower +0.23 μs
ericgray_response 730.43 K - 1.27x slower +0.29 μs
chungwong_response 655.58 K - 1.42x slower +0.45 μs
parse_map_int_fields 567.61 K - 1.63x slower +0.68 μs
And the code used for the benchmark:
defmodule Sanitizer do
def parse_map_int_fields(map) do
ints_map =
map
|> Enum.filter(fn {k, _v} -> Atom.to_string(k) |> String.starts_with?("int_") end)
|> Enum.filter(fn {_k, v} -> !is_integer(v) and !is_nil(v) end)
|> Enum.map(fn {key, val} ->
case Integer.parse(val) do
:error ->
{key, nil}
{parsed, _} ->
{key, parsed}
end
end)
|> Map.new()
Map.merge(map, ints_map)
end
def kokolegorille_response(map) do
Map.new(map, fn {_k, _v} = el ->
if has_to_be_sanitized?(el), do: sanitize(el), else: el
end)
end
defp has_to_be_sanitized?({k, v}) do
Atom.to_string(k) |> String.starts_with?("int_") and !is_integer(v) and !is_nil(v)
end
defp sanitize({k, v}) do
case Integer.parse(v) do
:error ->
{k, nil}
{parsed, _} ->
{k, parsed}
end
end
def chungwong_response(map) do
parse(map)
end
def parse(m) when is_map(m), do: parse(Map.to_list(m), [])
def parse([], acc), do: Enum.into(acc, %{})
def parse([{k, v} | t], acc), do: parse(t, prepend({to_string(k), v}, acc))
def prepend({"int_" <> _ = k, v}, acc) when is_binary(v),
do: [{String.to_atom(k), to_integer(Integer.parse(v))} | acc]
def prepend({k, v}, acc), do: [{String.to_atom(k), v} | acc]
def to_integer(:error), do: nil
def to_integer({parsed, _}), do: parsed
def ericgray_response(map) do
Enum.reduce(map, %{}, &do_parse_map_int_fields/2)
end
# Reconstruct or accumulate key value pairs into a new map
def do_parse_map_int_fields({key, value}, map) do
case starts_with_int?(key) do
true ->
Map.put(map, key, parse_value(value))
false ->
Map.put(map, key, value)
end
end
# Boolean function returns true if field is int
def starts_with_int?(key) do
key
|> Atom.to_string()
|> String.starts_with?("int_")
end
# Parses the value of int field, if not an integer return nil
def parse_value(value) do
case Integer.parse(value) do
{value, _} -> value
:error -> nil
end
end
def mnussbaumer_response(map) do
reshape_map_int_fields(map)
end
@spec reshape_map_int_fields(map) :: map
def reshape_map_int_fields(map) when is_map(map),
do:
Enum.reduce(map, %{}, fn {k, v}, acc ->
Map.put(acc, k, maybe_reshape(k, v))
end)
@spec maybe_reshape(atom, term) :: integer | term
def maybe_reshape(k, v) when is_atom(k) and not is_integer(v) and not is_nil(v) do
case Atom.to_string(k) do
<<"int_", _>> ->
case Integer.parse(v) do
{new_val, _} -> new_val
_ -> nil
end
_ ->
v
end
end
def maybe_reshape(_, v), do: v
def gregvaughn_response(map) do
for {k, v} <- map, match?("int" <> _, Atom.to_string(k)), into: map do
case Integer.parse(v) do
{int, _} -> {k, int}
_other -> {k, nil}
end
end
end
end
test_map = %{
int_a: "1",
int_b: "not_a_int!",
str_a: "This is a string field",
str_b: "Another string field",
fl_a: 0.0
}
Benchee.run(%{
"parse_map_int_fields" => fn -> Sanitizer.parse_map_int_fields(test_map) end,
"kokolegorille_response" => fn -> Sanitizer.kokolegorille_response(test_map) end,
"chungwong_response" => fn -> Sanitizer.chungwong_response(test_map) end,
"ericgray_response" => fn -> Sanitizer.ericgray_response(test_map) end,
"mnussbaumer_response" => fn -> Sanitizer.mnussbaumer_response(test_map) end,
"gregvaughn_response" => fn -> Sanitizer.gregvaughn_response(test_map) end
})
eksperimental
you can optimize it even more:
def maybe_reshape(k, v) when is_atom(k) and is_binary(v) do
rmoretto
I agree with you that to be more precise with the parsing the "" remainder should be match, but in my use case is better that any integer is saved than none.
I have run the benchmark with the code that you shared, only changing the line {v_int, ""} to {v_int, _} to match the other functions, here are the results:
Name ips average deviation median 99th %
mnussbaumer_response 960.24 K 1.04 μs ±2517.65% 0.94 μs 1.80 μs
gregvaughn_response 868.02 K 1.15 μs ±1919.57% 1.03 μs 2.04 μs
kokolegorille_response 827.66 K 1.21 μs ±2517.58% 1.09 μs 1.65 μs
ericgray_response 724.15 K 1.38 μs ±1604.94% 1.26 μs 2.50 μs
chungwong_response 656.80 K 1.52 μs ±1600.17% 1.38 μs 2.71 μs
parse_map_int_fields 623.51 K 1.60 μs ±1424.02% 1.45 μs 2.52 μs
eksperimental_response 612.94 K 1.63 μs ±1509.09% 1.47 μs 2.92 μs
Comparison:
mnussbaumer_response 960.24 K
gregvaughn_response 868.02 K - 1.11x slower +0.111 μs
kokolegorille_response 827.66 K - 1.16x slower +0.167 μs
ericgray_response 724.15 K - 1.33x slower +0.34 μs
chungwong_response 656.80 K - 1.46x slower +0.48 μs
parse_map_int_fields 623.51 K - 1.54x slower +0.56 μs
eksperimental_response 612.94 K - 1.57x slower +0.59 μs
I guess that the :maps.from_list() is too expensive to be worth in this specific case?
dimitarvp
Naive solution on a first try:
defmodule IntMapSanitizer do
@test_map_1 %{
greet: "Hirrow",
int_a: "17",
int_b: "gibberish",
int_c: "428",
farewell: "Aufwiedersehen"
}
defp is_int_name?(k) when is_atom(k) do
k
|> Atom.to_string()
|> String.starts_with?("int_")
end
defp should_keep?(v) when is_nil(v) or is_integer(v), do: false
defp should_keep?(_), do: true
defp parse_to_int_or_nil(v) do
case Integer.parse(v) do
:error ->
nil
{parsed, _} ->
parsed
end
end
def sanitize(%{} = map) do
for {k, v} <- map, is_int_name?(k), should_keep?(v), reduce: map do
acc -> Map.update(acc, k, nil, &parse_to_int_or_nil(&1))
end
end
def test1() do
sanitize(@test_map_1)
end
end
Try in iex:
iex(1)> IntMapSanitizer.test1()
%{
farewell: "Aufwiedersehen",
greet: "Hirrow",
int_a: 17,
int_b: nil,
int_c: 428
}
Haven’t measured but for comprehensions with filters and the reduce option have always been fairly fast in my experience.
Maybe if you make a public GitHub repo then we can make a benchmarking arena. 







