You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by aledsage <gi...@git.apache.org> on 2015/11/13 00:55:43 UTC

[GitHub] incubator-brooklyn pull request: Improve performance tests

GitHub user aledsage opened a pull request:

    https://github.com/apache/incubator-brooklyn/pull/1027

    Improve performance tests

    Our performance tests need a much bigger overhaul. However, these changes make them a bit more usable - and makes them record performance results in a local file which is better than nothing.
    
    - Extracts code from AbstractPerformanceTest into PerformanceMeasurer, making it more configurable and usable from more places.
    - Adds a default persister to write files to `~/brooklyn-performance/` (while we figure out where to write those to in anger).


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/aledsage/incubator-brooklyn tests/performance

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-brooklyn/pull/1027.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1027
    
----
commit 07f7ede8d36ed26760ad9948408dcc4aca6f0ee8
Author: Aled Sage <al...@gmail.com>
Date:   2015-11-12T23:52:11Z

    Improve performance tests
    
    - Extracts code from AbstractPerformanceTest into PerformanceMeasurer,
      making it more configurable and usable from more places.
    - Adds a default persister to write files to ~/brooklyn-performance/
      (while we figure out where to write those to in anger).

commit 27a55bc5cf719caa4e30b3a4de465a04484c3355
Author: Aled Sage <al...@gmail.com>
Date:   2015-11-12T23:53:15Z

    Delete duplicate testFilePermissionsPerformance
    
    This is already covered by 
    FilePersistencePerformanceTest.testFileBasedStoreObjectPuts

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Improve performance tests

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1027#discussion_r44742625
  
    --- Diff: usage/test-support/src/main/java/org/apache/brooklyn/test/performance/MeasurementOptions.java ---
    @@ -0,0 +1,135 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.brooklyn.test.performance;
    +
    +import static org.testng.Assert.assertFalse;
    +import static org.testng.Assert.assertNotNull;
    +import static org.testng.Assert.assertTrue;
    +
    +import java.io.File;
    +import java.util.concurrent.CountDownLatch;
    +
    +import org.apache.brooklyn.util.time.Duration;
    +import org.apache.commons.io.FileUtils;
    +
    +import com.google.common.annotations.Beta;
    +import com.google.common.base.Objects;
    +
    +/**
    + * For building up a description of what to measure.
    + * 
    + * See {@link PerformanceTestUtils#measure(MeasurementOptions)}.
    + */
    +@Beta
    +public class MeasurementOptions {
    --- End diff --
    
    `MeasurementOptions` is a cryptic name.  `PerformanceTestOptions` ? or `PerformanceTestDescriptor` (as some aren't optional) ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Improve performance tests

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1027#discussion_r44742652
  
    --- Diff: usage/test-support/src/main/java/org/apache/brooklyn/test/performance/MeasurementResult.java ---
    @@ -0,0 +1,62 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.brooklyn.test.performance;
    +
    +import java.util.List;
    +
    +import org.apache.brooklyn.util.time.Duration;
    +
    +import com.google.common.annotations.Beta;
    +import com.google.common.base.Objects;
    +
    +/**
    + * The results of a performance test.
    + * 
    + * See {@link PerformanceTestUtils#measure(MeasurementOptions)}.
    + */
    +@Beta
    +public class MeasurementResult {
    --- End diff --
    
    and `PerformanceTestResult` ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Improve performance tests

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1027#discussion_r44742691
  
    --- Diff: usage/test-support/src/main/java/org/apache/brooklyn/test/performance/PerformanceMeasurer.java ---
    @@ -0,0 +1,154 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.brooklyn.test.performance;
    +
    +import static org.testng.Assert.fail;
    +
    +import java.util.Date;
    +import java.util.List;
    +import java.util.concurrent.Future;
    +import java.util.concurrent.TimeUnit;
    +
    +import org.apache.brooklyn.util.exceptions.Exceptions;
    +import org.apache.brooklyn.util.time.Duration;
    +import org.apache.brooklyn.util.time.Time;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import com.google.common.annotations.Beta;
    +import com.google.common.base.Stopwatch;
    +import com.google.common.collect.Lists;
    +
    +/**
    + * For running simplistic performance tests, to measure the number of operations per second.
    + * 
    + * With a short run, this is "good enough" for eye-balling performance, to spot if it goes 
    + * horrendously wrong. 
    + * 
    + * However, good performance measurement involves much more warm up (e.g. to ensure java HotSpot 
    + * optimisation have been applied), and running the test for a reasonable length of time.
    + * 
    + * Longevity tests are also important for to check if object creation is going to kill
    + * performance in the long-term, etc.
    + */
    +@Beta
    +public class PerformanceMeasurer {
    +
    +    private static final Logger LOG = LoggerFactory.getLogger(PerformanceMeasurer.class);
    +
    +    /**
    +     * Runs a performance test. Repeatedly executes the given job. It measuring either the time it takes for
    +     * many iterations, or the number of iterations it can execute in a fixed time.
    +     */
    +    public static MeasurementResult measure(MeasurementOptions options) {
    --- End diff --
    
    `run(PerformanceTestDescriptor)` ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Improve performance tests

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1027#discussion_r44764144
  
    --- Diff: usage/test-support/src/main/java/org/apache/brooklyn/test/performance/PerformanceMeasurer.java ---
    @@ -0,0 +1,154 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.brooklyn.test.performance;
    +
    +import static org.testng.Assert.fail;
    +
    +import java.util.Date;
    +import java.util.List;
    +import java.util.concurrent.Future;
    +import java.util.concurrent.TimeUnit;
    +
    +import org.apache.brooklyn.util.exceptions.Exceptions;
    +import org.apache.brooklyn.util.time.Duration;
    +import org.apache.brooklyn.util.time.Time;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import com.google.common.annotations.Beta;
    +import com.google.common.base.Stopwatch;
    +import com.google.common.collect.Lists;
    +
    +/**
    + * For running simplistic performance tests, to measure the number of operations per second.
    + * 
    + * With a short run, this is "good enough" for eye-balling performance, to spot if it goes 
    + * horrendously wrong. 
    + * 
    + * However, good performance measurement involves much more warm up (e.g. to ensure java HotSpot 
    + * optimisation have been applied), and running the test for a reasonable length of time.
    + * 
    + * Longevity tests are also important for to check if object creation is going to kill
    + * performance in the long-term, etc.
    + */
    +@Beta
    +public class PerformanceMeasurer {
    --- End diff --
    
    Agree with your other renames.
    
    However `TestRunner` is a technical term in testng (http://testng.org/javadoc/org/testng/TestRunner.html) so that doesn't seem like the right name. I'll stick with the clumsy-but-accurate `PerformanceMeasurer` for now, marked as `@Beta`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Improve performance tests

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1027#issuecomment-156490614
  
    Incorporated comments (except left the `PerformanceMeasurer` class name). Merging now.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Improve performance tests

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-brooklyn/pull/1027


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Improve performance tests

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1027#issuecomment-156295320
  
    good improvement; optional suggestion on the naming strategy


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Improve performance tests

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1027#discussion_r44742683
  
    --- Diff: usage/test-support/src/main/java/org/apache/brooklyn/test/performance/PerformanceMeasurer.java ---
    @@ -0,0 +1,154 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.brooklyn.test.performance;
    +
    +import static org.testng.Assert.fail;
    +
    +import java.util.Date;
    +import java.util.List;
    +import java.util.concurrent.Future;
    +import java.util.concurrent.TimeUnit;
    +
    +import org.apache.brooklyn.util.exceptions.Exceptions;
    +import org.apache.brooklyn.util.time.Duration;
    +import org.apache.brooklyn.util.time.Time;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import com.google.common.annotations.Beta;
    +import com.google.common.base.Stopwatch;
    +import com.google.common.collect.Lists;
    +
    +/**
    + * For running simplistic performance tests, to measure the number of operations per second.
    + * 
    + * With a short run, this is "good enough" for eye-balling performance, to spot if it goes 
    + * horrendously wrong. 
    + * 
    + * However, good performance measurement involves much more warm up (e.g. to ensure java HotSpot 
    + * optimisation have been applied), and running the test for a reasonable length of time.
    + * 
    + * Longevity tests are also important for to check if object creation is going to kill
    + * performance in the long-term, etc.
    + */
    +@Beta
    +public class PerformanceMeasurer {
    --- End diff --
    
    `PerformanceTestRunner` ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---