Changes like LUCENE-5634 make it clear that the default AttributeFactory stuff has a very high cost: weakmaps/reflection/etc.

The problem are not the weak maps and reflections. The reason why it is expensive is the fact that all attribute instances have to be put into the 2 LinkedHashMaps on creating the TokenStream. I just repeat: It is not the refection! We had this discussion already back 5 years ago with Michael Busch!

In addition, the AttributeFactory itsself has less impact (this was already tested while developing it in 2.9). This is why the weak maps are there - so it is fast, the only reflection ever happens is: Class#newInstance() is cheap in recent Java versions, the speed difference in micro benchmarks is small, as fast as a native new.

So I disagree with removing the default AttributeFactory, we still need it for non-default attributes, so: The simple workaround would be to use TOKEN_ATTRIBUTE_FACTORY instead, which falls back to the default one for unknown attributes.

I agree with clearAttributes(), but this should be solved with TOKEN_ATTRIBUTE_FACTORY , too.

Easy and simple one-line patch.

This uses the Token class as attributes impl, which supports:

public class Token extends CharTermAttributeImpl 
                   implements TypeAttribute, PositionIncrementAttribute,
                              FlagsAttribute, OffsetAttribute, PayloadAttribute, PositionLengthAttribute {

Strangely, this test fails:

[junit4] Tests with failures:
[junit4]   - org.apache.lucene.analysis.TestGraphTokenizers.testMockGraphTokenFilterOnGraphInput
[junit4]

So this one seems to catch some bug in Token.java or the test does not work with this attribute impl (maybe it copies/clones in a wrong way).

I found the bug: Token implemented PositionLengthAttribute but missed to implement all the clone/copyTo/equals/... shit. I willheavy commit that, because its a bug.

I created a subtask: LUCENE-5639

In analysis module, TestWikipediaTokenizer also fails, we have to dig. I don't understand the failure.

I also created another subtask to clean up the Token class and remove stupid copy-ctors and all those reinit() methods. Unmaintainable! LUCENE-5640

I benchmarked this patch against trunk with luceneutil's TestAnalyzerPerf.java.

I ran 3 runs of it against both trunk and patch (because its quite noisy), but I think it shows a trend:
especially for ones like Shingles doing lots of saveState()/restoreState() and ones like n-grams calling clearAttributes() many times.

Times are reported in milliseconds to analyze en-wiki linedocs file: lower is better.

About the patch: there are more Tokenizers in the analysis/ module referring to DEFAULT_ATTRIBUTE_FACTORY directly today (because they all have ctors to pass in a custom factory). So we should fix those too.

Updated patch: I added it to other tokenizers, i also got MockTokenizer using it by default and randomize between DEFAULT and TOKEN attribute factories in TestRandomChains.

There were some test failures in TestPayloads, because they asserted PayloadAttribute was not present. I just removed the assertions.

I will look for other failures and also inspect FreqProxTermsWriter to ensure it is optimized for the case where payloadAtt != null but returns null every time, and benchmark indexing somehow with the patch.

Core tests are passing. Some analysis/common tests fail:

   [junit4] Tests with failures:
   [junit4]   - org.apache.lucene.analysis.wikipedia.WikipediaTokenizerTest.testRandomStrings
   [junit4]   - org.apache.lucene.analysis.wikipedia.WikipediaTokenizerTest.testRandomHugeStrings
   [junit4]   - org.apache.lucene.analysis.miscellaneous.TestWordDelimiterFilter.testRandomStrings
   [junit4]   - org.apache.lucene.analysis.miscellaneous.TestWordDelimiterFilter.testRandomHugeStrings
   [junit4]   - org.apache.lucene.analysis.path.TestPathHierarchyTokenizer.testRandomStrings
   [junit4]   - org.apache.lucene.analysis.path.TestPathHierarchyTokenizer.testRandomHugeStrings
   [junit4]   - org.apache.lucene.analysis.path.TestReversePathHierarchyTokenizer.testRandomStrings
   [junit4]   - org.apache.lucene.analysis.path.TestReversePathHierarchyTokenizer.testRandomHugeStrings

Looking into these now...

Now that tests are sorted out: here is an updated patch.

Updated patch: all tests (and precommit) pass.

I benchmarked indexing with various types of fields, the change actually gives a speedup even to StringFields (i suppose from faster clearAttributes?)

This is ready to commit.

+1 We shoudl fix the Token subtask, too. Most of work is done, but maybe we split Token up.

And: We should remove FlagsAttrbute!

Commit 1592353 from Robert Muir in branch 'dev/trunk'
[ https://svn.apache.org/r1592353 ]

LUCENE-5638: pack the core attributes into one impl by default

Commit 1592357 from Robert Muir in branch 'dev/branches/branch_4x'
[ https://svn.apache.org/r1592357 ]

LUCENE-5638: pack the core attributes into one impl by default

More improvements to the DefaultAttributeFactory are developed in the subtask: LUCENE-5640 (using Java 7 MethodHandles)