[JENKINS-55900] Java memory mishandling identified

[timja]

I was running Jenkins in docker container lts-alpine on low memory instance on cloud. At the beginning it looked working well. But after some period of time container stopped. I started it again and it stopped after some time again even I was doing nothing there.

I realized that OutOfMemory happens there. I decided to investigate locally using YourKit.

It showed that hudson.model.TimeSeries.update() is constantly allocating memory in Java heap.

Of course GC is handling that. But looking at the code I think memory allocation can be handled much better.

public void update(float newData) {
    float data = history[0]*decay + newData*(1-decay);

    float[] r = new float[Math.min(history.length+1,historySize)];
    System.arraycopy(history,0,r,1,Math.min(history.length,r.length-1));
    r[0] = data;
    history = r;
}

As for me this code looks like updating fixed sized queue. And each time new array created when only one element should be added. This is bad approach.

I would suggest to use EvictingQueue from Google Guava or CircularFifoQueue from Apache Commons, or at least LinkedList with removing last element if size exceeds. 

Also I found very old bugs related to this. They were closed without any analysis: 

1. JENKINS-12333
2. JENKINS-8330

---

Originally reported by ybulatnikov, imported from: Java memory mishandling identified

• assignee: ybulatnikov
• status: Open
• priority: Minor
• resolution: Unresolved
• imported: 2022/01/10

[timja]

oleg_nenashev:

ybulatnikov would you be interested to propose a pull request for it?

[timja]

oleg_nenashev:

And thanks a lot for the report. It would be a good improvement

 

[timja]

ybulatnikov:

I have created pull request https://github.com/jenkinsci/jenkins/pull/3878

I'm not sure why Jenkins cannot build it. I don't see anything wrong there. (Now I see in classic view hudson.remoting.RequestAbortedException. I'm not sure it is related to my changes.)

I used ArrayDeque for my solution. I found it more suitable. 

Also I see in JProfiler there Float.valueOf() is used because of my changes. It allocates some memory, but much less than before.

I think maybe it would be suitable to create own class that operates with primitive floats in same way as ArrayDeque ???

[timja]

ybulatnikov:

Oh, finally I see what's the point of creating new arrays constantly. 

@CopyOnWrite
private volatile float[] history;

Documentation of this annotation states: 

Represents fields that are protected for concurrency by the copy-on-write semantics.
Fields marked by this annotation always holds on to an immutable collection. A change to the collection must be done by first creating a new collection object, making changes, then replacing the reference atomically.
This allows code to access the field without synchronization, and greatly reduces the risk of dead-lock bugs.
The field marked with this annotation usually needs to be marked as volatile.

So this is not so trivial to fix that I thought. I've closed my pull request for now.

[timja]

danielbeck:

ybulatnikov It's unclear whether this issue describes a real problem, especially given your recent update. Could you please clarify?

[timja]

ybulatnikov:

danielbeck I noticed that this peace of code is allocating memory constantly. And small GC for young heap runs at least once per second. This happens even nothing is running on Jenkins. So I would like to improve the memory throughput. Maybe this is not really a bug. Maybe it would be small enhancement.