Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Purge JITServer caches sooner when memory is low #11189

Merged
merged 1 commit into from
Nov 18, 2020
Merged

Purge JITServer caches sooner when memory is low #11189

merged 1 commit into from
Nov 18, 2020

Conversation

EmanElsaban
Copy link
Contributor

Added variable OLD_AGE_UNDER_LOW_MEMORY to purge the Client cache at the server sooner
when memory is low. If a client didn't issue a request in more than 5 mins and memory is low, its
caches will be purged from the server.
issue: #11161

Signed-off-by: Eman Elsabban eman.elsaban1@gmail.com

@mpirvu
Copy link
Contributor

mpirvu commented Nov 16, 2020

I will review it shortly. Please change the title to "Purge JITServer caches sooner when memory is low"

@mpirvu mpirvu added the comp:jitserver Artifacts related to JIT-as-a-Service project label Nov 16, 2020
@mpirvu mpirvu added this to In progress in JIT as a Service via automation Nov 16, 2020
@mpirvu mpirvu self-assigned this Nov 16, 2020
@mpirvu mpirvu linked an issue Nov 16, 2020 that may be closed by this pull request
Purge JITServer caches sooner under memory pressure #11161
Closed
mpirvu
mpirvu reviewed Nov 16, 2020
View reviewed changes
Copy link
Contributor

@mpirvu mpirvu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see the inline comments.

runtime/compiler/runtime/JITClientSession.hpp Outdated Show resolved Hide resolved
runtime/compiler/runtime/JITClientSession.cpp Outdated Show resolved Hide resolved
runtime/compiler/runtime/JITClientSession.cpp Outdated Show resolved Hide resolved
runtime/compiler/runtime/JITClientSession.cpp Outdated Show resolved Hide resolved
@EmanElsaban EmanElsaban changed the title WIP: Added OLD_AGE_UNDER_LOW_MEMORY to purge caches sooner when memory is low WIP: Purge JITServer caches sooner when memory is low Nov 16, 2020
mpirvu
mpirvu reviewed Nov 17, 2020
View reviewed changes
runtime/compiler/control/J9Options.cpp Show resolved Hide resolved
runtime/compiler/control/J9Options.cpp Outdated Show resolved Hide resolved
runtime/compiler/runtime/JITClientSession.cpp Show resolved Hide resolved
runtime/compiler/runtime/JITClientSession.cpp Outdated Show resolved Hide resolved
runtime/compiler/runtime/JITClientSession.cpp Outdated Show resolved Hide resolved
runtime/compiler/runtime/JITClientSession.cpp Outdated Show resolved Hide resolved
@EmanElsaban EmanElsaban changed the title WIP: Purge JITServer caches sooner when memory is low Purge JITServer caches sooner when memory is low Nov 17, 2020
mpirvu
mpirvu reviewed Nov 17, 2020
View reviewed changes
runtime/compiler/control/J9Options.hpp Show resolved Hide resolved
runtime/compiler/runtime/JITClientSession.cpp Outdated Show resolved Hide resolved
runtime/compiler/control/J9Options.cpp Outdated Show resolved Hide resolved
@EmanElsaban
WIP: Added OLD_AGE_UNDER_LOW_MEMORY to purge caches sooner
390f344 
Added variable OLD_AGE_UNDER_LOW_MEMORY to purge the Client cache at the server sooner
when memory is low. If a client didn't issue a request in more than 5 mins and memory is low, its
cache will be purged from the server. I also added option oldAge for OLD_AGE and TimeBetweenPurges
for TIME_BETWEEN_PURGES. They can be specified using -xjit option.
issue: eclipse-openj9#11161

Signed-off-by: Eman Elsabban <eman.elsaban1@gmail.com>
mpirvu
mpirvu approved these changes Nov 17, 2020
View reviewed changes
Copy link
Contributor

@mpirvu mpirvu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
Could you please tests the implementation locally to make sure it works as designed? Jenkins testing won't show much in this case. Thanks

@mpirvu
Copy link
Contributor

mpirvu commented Nov 17, 2020

jenkins test sanity plinuxjit,xlinuxjit,zlinuxjit jdk11

@mpirvu
Copy link
Contributor

mpirvu commented Nov 18, 2020

jenkins test sanity plinuxjit,xlinuxjit jdk11

@mpirvu
Copy link
Contributor

mpirvu commented Nov 18, 2020

@EmanElsaban Tests have passed. I will merge this PR as soon as you confirm that local tests show behavior is as intended.

@EmanElsaban
Copy link
Contributor Author

EmanElsaban commented Nov 18, 2020 •
edited

@EmanElsaban Tests have passed. I will merge this PR as soon as you confirm that local tests show behavior is as intended.

I copied my jdk to acmeair benchmark and tested there. I had 2 acmeair clients connected to jitserver and I limited jitserver memory to 50MiB(low memory scenario). I started the 2 clients at the same time and I set oldAge=3000 , timeBetweenPurges=100 , oldAgeUnderLowMemory=500 . In the vlog for server I see the 500 value set and the data being purged(the purging is triggered):

JITServer: Server will purge session data for clientUID 14792167136674644815 of age 500
#JITServer: Server will purge session data for clientUID 14792167136674644815 of age 500
#JITServer: Server will purge session data for clientUID 14792167136674644815 of age 500
#JITServer: Server will purge session data for clientUID 14231492211954907412 of age 500
#JITServer: Server will purge session data for clientUID 14792167136674644815 of age 500

I also tested using java -version. I printed the values inside purgeOldDataIfNeeded to see if they are the same values as the ones specified in the command line, and they are, if the options are not used on CLI then they're set to default value. The values get printed on the server side. I also tried not specifying the values on server side and passing the options on the client side, in this case since the options on server side are not specified, the default values are set, so the values specified on the client dont take place.

@mpirvu
Copy link
Contributor

mpirvu commented Nov 18, 2020

I am merging this PR since all tests (Jenkins and local) have passed.

@mpirvu mpirvu merged commit 280eaae into eclipse-openj9:master Nov 18, 2020
JIT as a Service automation moved this from In progress to Done Nov 18, 2020
@mpirvu mpirvu mentioned this pull request Jan 19, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mpirvu mpirvu mpirvu approved these changes

Assignees

@mpirvu mpirvu

Labels
comp:jitserver Artifacts related to JIT-as-a-Service project
Projects
JIT as a Service
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Purge JITServer caches sooner under memory pressure
2 participants
@EmanElsaban @mpirvu