elastic / elastic/logstash

Java Conversion of Event Fields should be Lazy

Open
#7,803 6 comments 0 reactions 0 assignees View on GitHub
design performance improvements
Dominant language
Java
Stars
14.9k
Forks
3.5k
Avg merge
19h 14m
Merged PRs (30d)
63

Description

I think we have huge optimization potential when it comes to setting and retrieving data from an `Event`.

Currently this example in Ruby:

```ruby
e = LogStash::Event.new({
"foo" => "bar", "bar" => 1, "baz" => 1.0, TIMESTAMP => "2015-05-28T23:02:05.350Z"
})
e.get("foo")
```

causes the following things to happen under the hood:

#### Instantiation
* Convert `RubyHash` to `ConvertedMap`
* Create a BiValue for every entry in the `ConvertedMap`
* Create `Event` object with a bunch of additional `HashMap<>` etc.

#### Get
* Turn the key "foo" from a `RubyString` into a `String`
* Lookup on `Event` which creates an entry in the `PathCache`
* and some more objects in `Accessor`

-------------------

This is just wrong in my opinion and an extreme overhead memory wise (not that in the apache logs benchmark example you can easily attribute `50%+` of the GC activity to the above redundant steps).

IMO:

* No Java `Event` should be created unless we have a Java access to it or want to serialize it
* all operations on the `RubyEvent` should stay strictly in JRuby class space without any unnecessary instantiations/conversions
* We should only switch over to the current code once the `RubyEvent` has seen a Java type access

This would also allow for much more pinpointed optimizations. Currently, we're optimizing a code-path that is only used by PQ (Serialization) and Java(ish) plugins while artificially/needlessly slowing down all other paths.
I think it would be much easier to find ways of reducing the Java-Ruby barrier overhead if it only hits us in specific cases.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.