Uploaded image for project: 'MINA'
  1. MINA
  2. DIRMINA-838

Redundant AttributeKey allocation resulting in high garbage collector activity

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.3
    • Fix Version/s: 2.0.8
    • Component/s: Filter
    • Labels:
      None
    • Environment:
      java version "1.6.0_22"
      Java(TM) SE Runtime Environment (build 1.6.0_22-b04)
      Java HotSpot(TM) 64-Bit Server VM (build 17.1-b03, mixed mode)

      Description

      The classes CumulativeProtocolDecoder and ProtocolCodecFilter (there are others too, but these two are the important ones for our project) have members of type AttributeKey declared as:
      private final AttributeKey ENCODER = new AttributeKey(ProtocolCodecFilter.class, "encoder");

      As can be seen, these are not static, so each time ProtocolCodecFilter is created, AttributeKey objects get created as well, which in turn involves creation of a lot of String and char[] objects. jProfiler indicates that AttributeKey is accountable for creation of about 85% of all Strings and char[] in runs of our project, so it makes a significant difference for us.

      Is it OK for these attribute key objects to be singletons? If not, what would take to make them singletons?
      Some of other MINA filters have AttributeKeys as static members (e.g. MdcInjectionFilter, SslFilter).

      To summarize: the improvement being requested is to make the pre-defined AttributeKey objects static within classes such as CumulativeProtocolDecoder and ProtocolCodecFilter.

        Activity

        Hide
        elecharny Emmanuel Lecharny added a comment -

        Interestng question. It has been changed a while back to be static, but we gor some failures back then.

        Any reason why you have so any AttributeKeys stuck in memory ? They should be discarded as soon as the session is terminated.

        Show
        elecharny Emmanuel Lecharny added a comment - Interestng question. It has been changed a while back to be static, but we gor some failures back then. Any reason why you have so any AttributeKeys stuck in memory ? They should be discarded as soon as the session is terminated.
        Hide
        minisaw Platon Potapov added a comment -

        AttributeKeys do get discarded; MINA leaks no memory. But our project creates a lot of connections (thousands per second), so the overhead of creating/discarding many AttributeKeys is tangible. In case AttributeKeys do not need to be unique per ProtocolCodecFilter, this hurts performance for no benefit.

        However, I am not familiar with MINA well enough to assert that AttributeKeys can by design be singletons in this place - can they?
        You said there were failures with having these being static - could it have been because AttributeKeys were being incorrectly used elsewhere (a latent bug)?

        Show
        minisaw Platon Potapov added a comment - AttributeKeys do get discarded; MINA leaks no memory. But our project creates a lot of connections (thousands per second), so the overhead of creating/discarding many AttributeKeys is tangible. In case AttributeKeys do not need to be unique per ProtocolCodecFilter, this hurts performance for no benefit. However, I am not familiar with MINA well enough to assert that AttributeKeys can by design be singletons in this place - can they? You said there were failures with having these being static - could it have been because AttributeKeys were being incorrectly used elsewhere (a latent bug)?
        Hide
        elecharny Emmanuel Lecharny added a comment -

        I have to check what kind of errors I had when I made this field a singleton. I must admit we were more focused on sme other major bugs in MINA, so I cowardly reverted my change...

        In any case, we should be able to make this optionnal.

        i'll investigate this point this week.

        Show
        elecharny Emmanuel Lecharny added a comment - I have to check what kind of errors I had when I made this field a singleton. I must admit we were more focused on sme other major bugs in MINA, so I cowardly reverted my change... In any case, we should be able to make this optionnal. i'll investigate this point this week.
        Hide
        elecharny Emmanuel Lecharny added a comment -

        I have no idea why we had some error when we switched to static, but right now, we don't have those errors anymore.

        Changed the AttributeKey to be static, as suggested.

        Fixed with commit ba995db5e653307dd0bf6dc7ba5deeb903f83366

        Show
        elecharny Emmanuel Lecharny added a comment - I have no idea why we had some error when we switched to static, but right now, we don't have those errors anymore. Changed the AttributeKey to be static, as suggested. Fixed with commit ba995db5e653307dd0bf6dc7ba5deeb903f83366

          People

          • Assignee:
            Unassigned
            Reporter:
            minisaw Platon Potapov
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development