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.

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

Querying toggles, that are not definied in the property file, take 10 times longer #43

Closed
finsterwalder opened this Issue Jun 6, 2013 · 9 comments

Comments

Projects
None yet
2 participants
@finsterwalder

finsterwalder commented Jun 6, 2013

I just noticed, that togglz takes about 10 times as much time, when I query a feature that is not specified in my featuretoggles.properties file, compared to querying a feature that is specified in my featuretoggles.properties file.

When my featuretoggles.properties file looks like this:
FEATURE1=true
FEATURE2=false

FEATURE1.isActive() and FEATURE2.isActive() are answered 10 times faster than OTHER_FEATURE.isActive()

@chkal

This comment has been minimized.

Member

chkal commented Jun 6, 2013

Thanks for sharing this interesting observation. Which version of Togglz are you using?

I just had a quick look at the implementation and I don't see any obvious reason for this in the Togglz code. Perhaps the performance of the underlying java.util.Properties object is the cause? But this would be really weird.

Not sure what may cause this. What is the size of your feature enum?

@ghost

This comment has been minimized.

ghost commented Jun 6, 2013

You might also want to check the average of the query method when its running 1000000 times. This will make sure your performence test will be more accurate.-------- הודעה מקורית --------

Christian Kaltepoth :מ
Thu, Jun 6, 2013 6:45 PM :שלח
togglz/togglz :אל
:העתק

Re: [togglz] Querying toggles, that are not definied in the property file, take 10 times longer (#43) :נושא

Thanks for sharing this interesting observation. Which version of Togglz are you using?

I just had a quick look at the implementation and I don't see any obvious reason for this in the Togglz code. Perhaps the performance of the underlying java.util.Properties object is the cause? But this would be really weird.

Not sure what may cause this. What is the size of your feature enum?


Reply to this email directly or view it on GitHubhttps://github.com//issues/43#issuecomment-19053422.

@finsterwalder

This comment has been minimized.

finsterwalder commented Jun 6, 2013

I did measure with 100000 runs. The timings were very reproducible.
On my machine ~250ms for a toggle that is set.
~2700ms when the toggle was not set.
I built my own very simple Properties-based toggles implementation. That does not show this behaviour. So Properties.getProperty does not show a different performance when getting a key that is not present.

I'm using:
org.togglz
togglz-core
1.1.0.Final

@ghost

This comment has been minimized.

ghost commented Jun 6, 2013

Im also gussing you ran 2 different runs and not o.e method after another-------- הודעה מקורית --------

Malte Finsterwalder :מ
Thu, Jun 6, 2013 7:11 PM :שלח
togglz/togglz :אל
Abramovitch, Eli :העתק

Re: [togglz] Querying toggles, that are not definied in the property file, take 10 times longer (#43) :נושא

I did measure with 100000 runs. The timings were very reproducible.
On my machine ~250ms for a toggle that is set.
~2700ms when the toggle was not set.
I built my own very simple Properties-based toggles implementation. That does not show this behaviour. So Properties.getProperty does not show a different performance when getting a key that is not present.

I'm using:
org.togglz
togglz-core
1.1.0.Final


Reply to this email directly or view it on GitHubhttps://github.com//issues/43#issuecomment-19055864.

@chkal

This comment has been minimized.

Member

chkal commented Jun 6, 2013

@finsterwalder: Thanks for sharing these details. I'll try to reproduce this myself. Perhaps I'll find out what is causing this.

chkal added a commit that referenced this issue Jun 7, 2013

@chkal

This comment has been minimized.

Member

chkal commented Jun 7, 2013

I just tried to reproduce this issue with a simple test:

https://github.com/togglz/togglz/blob/master/core/src/test/java/org/togglz/core/repository/file/FileBasedRepositoryPerformanceTest.java

But I'm getting performance metrics just like I expected them. Reading existing features is more than twice as fast as missing ones. This is reasonable because Togglz has read multiple properties in case a feature is found in the file.

For 100000 getFeature() calls:

  • Existing feature: 1107 ms
  • Missing feature: 357 ms
@ghost

This comment has been minimized.

ghost commented Jun 9, 2013

since performence testings are very affected from the process starting, i suggests you'd upgrade your test with a 2nd loop and use nanoTime():

    for (int i = 0; i < 10; i++) {
        long start = System.nanoTime();
        for (long j = 0; j < 100000; j++) {
            repository.getFeatureState(feature);
        }
        long time = System.nanoTime() - start;
        System.out.println("Time for " + feature.name() + ": " + time);
    }

this helps exposed what the real performance is:
you can easily see smth like:
9159084 --> this is the start so it's slow
3457302 --> this still doesn't reflect the real performence
2750492 --> now we are talking....
2753615
2750491
2755846
2754061
2751830
2750938
2749153

for me simple properties not related to togglz i got 30% slowdown on getting a key that exists:
key exists:
11036771 ns
key doesn't exists:
8798539 ns

Hope this helps

@finsterwalder

This comment has been minimized.

finsterwalder commented Jun 10, 2013

My own, very simple implementation (also based on java.util.Properties) does 100000 requests in 65 to 100ms. Measured in a nested loop, like eliab described above. The first 100000 take 140ms.
It does not matter, whether a feature is definied in the file and as such present in the properties or not.

An extra small performance improvement I did: I check the lastModified timestamp of the properties file at most every 100ms. That makes a huge difference in the very tight loop.

@chkal

This comment has been minimized.

Member

chkal commented Jun 11, 2013

@finsterwalder:

Thanks a lot for your help with this issue. You are totally correct. The check of the last modified date seems to be very expensive. I implemented the performance improvement you described. The last modification date is now checked at most every second. The results for 100.000 requests to the state repository:

  • Results without the optimization:
    • ~600ms for features present in the file
    • ~300ms for features that are not in the file
  • After doing this optimization:
    • ~25ms for both cases

I'll close this issue. Thanks a lot. :)

@chkal chkal closed this Jun 11, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment