Bug 191777 - 5s - MIMESupport.findMIMEType() - layer based registration of mime type
5s - MIMESupport.findMIMEType() - layer based registration of mime type
Status: VERIFIED FIXED
Product: platform
Classification: Unclassified
Component: Filesystems
7.0
All All
: P2 (vote)
: 7.2
Assigned To: Jaroslav Tulach
issues@platform
EXCEPTIONS_REPORT perf-profileme
: API_REVIEW_FAST, PERFORMANCE
: 193720 197177 (view as bug list)
Depends on: 207960 207959
Blocks: 207393 224007
  Show dependency treegraph
 
Reported: 2010-11-10 07:44 UTC by TobiasKranz
Modified: 2012-12-19 13:11 UTC (History)
20 users (show)

See Also:
Issue Type: DEFECT
:


Attachments
nps snapshot (20.77 KB, application/nps)
2010-11-10 07:44 UTC, TobiasKranz
Details
Another snapshot showing 0.5s-1s waisted in mime type recognition on start (9.75 KB, application/x-nps)
2012-01-06 15:47 UTC, Jaroslav Tulach
Details
Introducing @MimeResolver.Registration (1.75 KB, patch)
2012-01-10 11:00 UTC, Jaroslav Tulach
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description TobiasKranz 2010-11-10 07:44:14 UTC
Build: NetBeans IDE 7.0 M2 (Build 201010151251)
VM: OpenJDK Client VM, 19.0-b06, Java(TM) SE Runtime Environment, 1.6.0_23-ea-b01
OS: Windows XP

User Comments:
GUEST: abriendo netbeans

TobiasKranz: Startup

GUEST: Open the netbeans

GUEST: LowPerformance took 7906 ms.

GUEST: Abrir netbeans

GUEST: abriendo netbeans



Maximum slowness yet reported was 58158 ms, average is 21902
Comment 1 TobiasKranz 2010-11-10 07:44:19 UTC
Created attachment 102872 [details]
nps snapshot
Comment 2 Jaroslav Tulach 2010-11-10 08:12:20 UTC
AWT thread waits for Folder Instance which spends most of the time parsing MIME resolver XML files.
Comment 3 Jaroslav Tulach 2011-04-05 05:11:33 UTC
*** Bug 197177 has been marked as a duplicate of this bug. ***
Comment 4 Jaroslav Tulach 2011-05-24 06:49:40 UTC
*** Bug 193720 has been marked as a duplicate of this bug. ***
Comment 5 Jaroslav Tulach 2012-01-06 15:47:28 UTC
Created attachment 114686 [details]
Another snapshot showing 0.5s-1s waisted in mime type recognition on start

Snapshot from a NetBeans based application where start is delayed significantly by parsing existing mime type XML definitions.
Comment 6 Jaroslav Tulach 2012-01-10 11:00:36 UTC
Created attachment 114762 [details]
Introducing @MimeResolver.Registration
Comment 7 Jaroslav Tulach 2012-01-10 11:02:00 UTC
The primary need of the API change is to eliminate parsing of XML definition files during runtime.

I was trying to find balance between the simplicity of the annotation and flexibility to cover corner cases. The best way seems to offer some basic attributes (currently extension) and for complicated cases point directly to existing XML definitions (just process them during compile time).

If reviewers approve, I'll proceed with implementation.
Comment 8 Jesse Glick 2012-01-12 00:33:21 UTC
The patch is not really enough to see how you plan to use this for performance improvements.

Why is there any @Target other than PACKAGE? What would it mean for other targets? (The rare nondeclarative resolver can already be registered with @ServiceProvider, though after 10d30eec11f7 these are unlikely to work.)

What would the mimeType attribute represent when definition is set? After all, one resolver can define multiple MIME types.

I would probably rather see a full replacement for the current confused XML format, as [1] suggests, even if it just mirrors the DTD in the annotation structure; at least you could use Javadoc to understand the detailed meaning of each item, the processor could easily validate semantic aspects, and you could reuse a Java constant for the MIME type.

Solving bug #18910 with a richer MIME annotation would be nice, but the JLS currently forbids recursive annotation definitions - even if the usages are finite - so there is no way to embed boolean expressions as annotation structure. You could create a small external DSL, however:

@MIMEResolver(mimeType="text/x-php5", position=99000, condition="name('*.php') || name('*.phtml') || !name('*.java') && !name('*.rb') && !name('*.rhtml') && (pattern('<?php', 255) || pattern('<?php', 4000) && (pattern('<HTML>', 255, true) || pattern('<!DOCTYPE HTML', 255, true))")

which is less redundant than the current php-project-mime-resolver.xml and arguably more readable. A more typical usage would be something like

@MIMEResolver(mimeType="text/x-oql", position=945, condition="name('*.oql')")

or

@MIMEResolver(mimeType="text/x-hibernate-cfg+xml", position=1500, condition="name('*.xml') && doctype('-//Hibernate/Hibernate Configuration DTD 3.0//EN')")

Once again I find myself wishing that small Java classes with few dependencies could be loaded with as little overhead as, say, Lisp interpreters parsing small s-expressions. I suppose we could use Rhino but this adds a significant new dependency to the whole system.


[1] http://wiki.netbeans.org/MoreDeclarativeRegistrationUsingAnnotations#MIME_resolvers
Comment 9 skygo 2012-01-14 20:40:57 UTC
Hi (thanks jesse to cc me to this issue)

IMHO if the xml parser is faulty the new annotation should not allow to refer to the xml definition.

If smooth transition is needed I would see a @Deprecated  annotation on the definition if there is a need for the rich definition with condition or definition to be complete.
@MIMEResolver(mimeType="...", position=..., condition="...", definition="..." )



=feedback.
I was investigating the mimeresolver xml files of main-silver netbeans source(before Jesse and Jaroslav post):

I wanted to look if a common "workflow"  may arise to limit to a set of annotation with no complex condition. It leads to too many. (Multiple annotation not a good approach IMO).

I saw also a user at least using xml comment in the mime resolver file (for hexa and mask matching see [1]). New approach should somehow give this possibility.





[1] 
http://hg.netbeans.org/core-main/file/46c65a13096e/cnd/src/org/netbeans/modules/cnd/resources/mime-resolver-hex-based.xml
Comment 10 Jaroslav Tulach 2012-01-25 16:26:28 UTC
OK, thanks for your advices. I've created branch marshall-mime-resolvers-191777 in ergonomics repository. And here is the bulk of my today's changes:
http://hg.netbeans.org/ergonomics/rev/cc381ddeb20b

To sum up. There is @MIMEResolver.Registration(resource="../the-definition.xml") that is fully backward compatible, yet more effective.

There is @MIMEResolver.ExtensionRegistration for the simple case when one needs just extension mapping. 

I've also created@MIMEResolver.NamespaceRegistration with the intent to make sure the apisupport's File Type wizard can always generate annotations, but I have not needed such annotation yet. Thus it is not finished.

I have not specified any @Target, so these annotations can be used on any element.

I am going to measure the effect of these changse on JDev startup and if OK, I'd like to merge early next week (together with bug 207219).
Comment 11 Jaroslav Tulach 2012-01-30 14:08:15 UTC
Non-XML resolvers rewritten: http://hg.netbeans.org/ergonomics/rev/e7d30459a7f9
Comment 12 Jaroslav Tulach 2012-01-30 19:15:06 UTC
@NamespaceRegistration and rewrite of the rest of resolvers:
http://hg.netbeans.org/ergonomics/rev/13ce4ffe5acb

Will try to integrate tomorrow (after ergonomics build finally succeeds at least once).
Comment 13 Jesse Glick 2012-01-30 22:57:33 UTC
Do not really like @Registration - does not address the limitations or confused design of the current DTD - but I guess that is all we are getting for now.


(In reply to comment #10)
> I have not specified any @Target, so these annotations can be used on any
> element.

[JG01] For consistency with other annotations not bound to a particular Java type, and to avoid being misleading, the target should be PACKAGE.


[JG02] A hint in apisupport.refactoring is recommended. It is not usually hard, but I can also do this if you prefer. It should detect any XML file registered in Services/MIMEResolver/ and offer to rewrite to one of the @MIMEResolver.* annotations, to be placed on package-info.java in the same package as the current resolver.


[JG03] In FileElement.java you seem to be basically reimplementing Java serialization. Please consider just making the top-level structure class implement Serializable and use Object-valued file attributes. This would simplify the implementation considerably and make it much easier to do things like add (optional) fields later. Runtime performance might not be quite as good as a hand-written binary protocol, but still close I think, and certainly far better than XML parsing.


[JG04] instanceClass=MIMEResolver should rather be instanceOf.


[JG05] @Registration Javadoc refers to {@code value} where it should say {@code resource}.
Comment 14 Jaroslav Tulach 2012-01-31 07:59:04 UTC
Re. JG01 - the file type wizard will generate @MimeResolver.xyzRegistration together with bug 207219 @DataObject.Registration next to each other on a DataObject subclass. Thus the target has to be at least TYPE. Requiring just TYPE is however too restricting. Thus I decided to not specify the target at all.

Re. JG03 - I considered to make all elements externalizable. The result would be similar, just the encoded "bytes" value would be longer (it would contain prologue for description of MimeResolverImpl class). Thus I stick with current shorter version.

Re. JG04 - the primary purpose is to prevent creating instances of the resolver when somebody queries different class, which instanceClass attribute does well.

Re. JG05 - 6a580a6a7754

Re. JG02 - I've just looked there. #1 - I've already converted all the hints in distribution manually. #2 - there is not unit test support for layer hints. I leave it up to you.
Comment 15 Jaroslav Tulach 2012-01-31 10:38:13 UTC
Integrated
http://hg.netbeans.org/ergonomics/rev/dc417bdb3643
now let's see what gets broken.
Comment 16 Jesse Glick 2012-01-31 16:28:28 UTC
(In reply to comment #14)
> Re. JG01 - the file type wizard will generate @MimeResolver.xyzRegistration
> together with bug 207219 @DataObject.Registration next to each other on a
> DataObject subclass. Thus the target has to be at least TYPE.

No, it just means that it needs to use CreateModifiedFiles.packageInfo.

I would consider this a TCR since all other annotations not tied to a Java type use target=PACKAGE, so I will file a blocking issue.

> Re. JG03 - I considered to make all elements externalizable.

That is no better than hand-coding the representation. Using Serializable makes the representation of fields automatic. Of course major refactorings are not feasible when using the same serializable classes but it is easy to deprecate them and introduce new serializable classes.

> the encoded "bytes" value would be longer (it would contain
> prologue for description of MimeResolverImpl class)

Yes, it would be a few bytes longer; did you measure this actually mattering?

> Re. JG04 - the primary purpose is to prevent creating instances of the resolver
> when somebody queries different class, which instanceClass attribute does well.

This is what instanceOf is for. instanceClass is for actually creating instances based on default ctor (when instanceCreate is missing). As a side effect for backwards compatibility it also implies an instanceOf of the same value if that is not explicitly specified, but it is misleading the instance which will really be created will not in fact be of that exact class. Really the presence of both instanceCreate and instanceClass attributes on the same file should be considered a user error and logged as a warning.

> I leave it up to you.

OK, I will file a blocking issue.
Comment 17 skygo 2012-02-27 16:34:56 UTC
Out of curiosity what is the performance now, with the new @MimeResolver.Registration ?
Comment 18 Jaroslav Tulach 2012-03-02 14:39:35 UTC
No new slowness reports for 7.2Dev. Hopefully it is better.
Comment 19 Jaroslav Tulach 2012-09-17 10:56:59 UTC
Are you wondering why you should convert your XML registration
http://netbeans.org/projects/platform/lists/dev/archive/2012-09/message/49
then follow the cookbook
http://netbeans.org/projects/platform/lists/dev/archive/2012-09/message/60
Comment 20 Jesse Glick 2012-09-17 15:16:15 UTC
(In reply to comment #19)
> follow the cookbook
> http://netbeans.org/projects/platform/lists/dev/archive/2012-09/message/60

Bug #207960 would automate this.


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