Yup the call is in the wrong place.
Calling trim only when adding a new bucket would be better
-Kelley

Sent from my iPhone

On May 17, 2013, at 12:31 PM, brharrington <notifications@github.com> wrote:

> In servo-core/src/main/java/com/netflix/servo/monitor/PeakRateCounter.java:
> 
> > +        increment(1L);
> > +    }
> > +
> > +    /**
> > +     * {@inheritDoc}
> > +     */
> > +    @Override
> > +    public void increment(long amount) {
> > +
> > +        long now = System.currentTimeMillis();
> > +        long ellapsedTime = now - buckets.get().getTimestamp();
> > +
> > +        long currentBucketKey = TimeUnit.SECONDS.convert(ellapsedTime, TimeUnit.MILLISECONDS);
> > +
> > +        incrementBucket(currentBucketKey, amount);
> > +        trimBuckets(currentBucketKey);
> The inline trim could make increment pretty expensive if it is being executed in a tight loop in the user's code. Looks like it would currently do to linear scans of the map for every increment. Thoughts?
> 
> —
> Reply to this email directly or view it on GitHub.