1612251 - Use Executors.newSingleThreadScheduledExecutor instead of Thread.sleep for Java Sampler

Status: RESOLVED FIXED

(Core :: Gecko Profiler, enhancement, P2)

Description

[Makoto Kato [:m_kato]]

Actually, we use Thread.sleep to get next sampling duration. But as long as old comment, this sampling min duration is 10ms (see https://searchfox.org/mozilla-central/rev/ad6234148892bc29bf08d736604ac71925138040/tools/profiler/core/platform.cpp#4192).

When I test Executors.newSingleThreadScheduledExecutor on old device (10 year's old Galaxy Nexus) instead of Thread.sleep, it can change to 1-2ms.

So we should use ScheduledExecutorService instead.

Comment 1

[Julien Wajsberg [:julienw]]

Hey Nazim, I think this is relevant to the other bug that aims to put the interval back a 1ms. It would make sense to tackle both bugs together. What do you think?

Comment 2

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Yeah, that's a good idea! We can land both of them together.

Makoto, it looks assigned to you. Do you mind if I take this one, if you are not working on it already?

Comment 3

[Makoto Kato [:m_kato]]

(In reply to Nazım Can Altınova [:canova][:canaltinova on phabricator] from comment #2)

Yeah, that's a good idea! We can land both of them together.

Makoto, it looks assigned to you. Do you mind if I take this one, if you are not working on it already?

I already have a patch, so it will be ready for review soon.

Comment 4

[Makoto Kato [:m_kato]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f29109776218180753c4c9ff727ae3ef571a4e5c

Comment 5

[Julien Wajsberg [:julienw]]

Hey makoto, try run seems green! Do you think you can push your patch for review?

Comment 6

[Makoto Kato [:m_kato]]

Attachment: Bug 1612251 - Use Executors.newSingleThreadScheduledExecutor to support <10ms sampling.

Actually, we use Thread.sleep to get next sampling duration. But as long as
old comment, this sampling min duration is 10ms. But when I test
Executors.newSingleThreadScheduledExecutor on old device (10 year's old
Galaxy Nexus) instead of Thread.sleep, it can use 2ms sampling duration.

So we should use ScheduledExecutorService instead.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1e06eab47c7a
Use Executors.newSingleThreadScheduledExecutor to support <10ms sampling. r=julienw

Comment 8

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/1e06eab47c7a