Bug 205575 - API review of adding ConstrainedBinaryIndexer.Registration.namePattern
API review of adding ConstrainedBinaryIndexer.Registration.namePattern
Status: RESOLVED FIXED
Product: editor
Classification: Unclassified
Component: Parsing & Indexing
7.1
All All
: P2 with 1 vote (vote)
: 7.2
Assigned To: Tomas Zezula
issues@editor
: API, API_REVIEW_FAST, PERFORMANCE, PLAN
Depends on:
Blocks: 205574
  Show dependency treegraph
 
Reported: 2011-11-26 11:27 UTC by Tomas Zezula
Modified: 2012-01-25 10:20 UTC (History)
4 users (show)

See Also:
Issue Type: TASK
:


Attachments
Patch file with logging (8.65 KB, application/octet-stream)
2011-11-26 11:27 UTC, Tomas Zezula
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tomas Zezula 2011-11-26 11:27:49 UTC
Created attachment 113544 [details]
Patch file with logging

Some indexers which are using the ConstrainedBinaryIndexer register very
expensive predicates. They register no required resource with combination with
expensive mime type (the XML mime recognizer needs to read the content of
file). For example the JSFBinaryIndexer only needs to test that resource.getNameExt().endsWith(".taglib.xml") so adding the cheap namePattern predicate improves performance.
Comment 1 Jaroslav Tulach 2011-11-28 07:22:45 UTC
Y01 I see no test. Given the magic with "content/unknown", it might tighten the semantics a bit.

+            final String[] parts = s.split(",");    //NOI18N
+            final StringBuilder sb = new StringBuilder();
+            for (String part : parts) {
+                sb.append(part);
+                sb.append('|'); //NOI18N
+            }

Y02 The above code basically does s.replace(',','|') - possibly you could change the annotation processor to use '|' as separator, then you could directly read the regexp in runtime without processing it.
Comment 2 Tomas Zezula 2011-11-28 10:49:01 UTC
>Given the magic with "content/unknown", it might tighten the semantics a bit.
Yes and no. The API was never part of any release so it's no change. It's documented.
Also NB API does not declare semantic compatibility.

Y02: OK
Comment 3 Jesse Glick 2011-11-28 17:45:34 UTC
I suppose this is now for 7.2.


JG01 after Y02 - or simply make namePattern be of type String (default ""). There is no purpose in writing e.g.

namePattern={".+[.]java", ".+[.]jsp"}

when you could just write e.g.

namePattern=".+[.]java|.+[.]jsp)"

and simplify it to

namePattern=".+[.](java|jsp)"
Comment 4 Tomas Zezula 2011-11-28 17:53:51 UTC
JG01: No problem.

I want to do the change into NB 7.1 if possible, it seems that the JSFBinaryIndexer becomes very slow when J2EE cluster is active. In the Java cluster the overhead of fo.getMimeType(String...) on XML file is small but it's 10 times slower when J2EE cluster is activated.
Comment 5 Tomas Zezula 2011-11-29 16:25:50 UTC
Fixed jet-main 955842fb95d4
Comment 6 Tomas Zezula 2011-11-29 16:41:00 UTC
Not approved to NB 7.1 by QA, scheduled for patch release.
Comment 7 Quality Engineering 2011-12-01 12:10:10 UTC
Integrated into 'main-golden'
Changeset: http://hg.netbeans.org/main-golden/rev/955842fb95d4
User: Tomas Zezula <tzezula@netbeans.org>
Log: #205575:API review of adding ConstrainedBinaryIndexer.Registration.namePattern


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo