This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
| Summary: | 5s - MIMESupport.findMIMEType() - layer based registration of mime type | ||
|---|---|---|---|
| Product: | platform | Reporter: | TobiasKranz |
| Component: | Filesystems | Assignee: | Jaroslav Tulach <jtulach> |
| Status: | VERIFIED FIXED | ||
| Severity: | normal | CC: | acolito, akiiller, apireviews, averri, azizur, daniel.gomes, dds.dhawal, dirkaholic, esmithbss, hideraldo.mello, hmichel, jglick, mchl, mhuebner, misterm, newbeewan2, skygo, sura, thilina01, zolta |
| Priority: | P2 | Keywords: | API_REVIEW_FAST, PERFORMANCE |
| Version: | 7.0 | ||
| Hardware: | All | ||
| OS: | All | ||
| Issue Type: | DEFECT | Exception Reporter: | 173637 |
| Bug Depends on: | 207960, 207959 | ||
| Bug Blocks: | 207393, 224007 | ||
| Attachments: |
nps snapshot
Another snapshot showing 0.5s-1s waisted in mime type recognition on start Introducing @MimeResolver.Registration |
||
|
Description
TobiasKranz
2010-11-10 07:44:14 UTC
Created attachment 102872 [details]
nps snapshot
AWT thread waits for Folder Instance which spends most of the time parsing MIME resolver XML files. *** Bug 197177 has been marked as a duplicate of this bug. *** *** Bug 193720 has been marked as a duplicate of this bug. *** 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.
Created attachment 114762 [details]
Introducing @MimeResolver.Registration
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. 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 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 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). Non-XML resolvers rewritten: http://hg.netbeans.org/ergonomics/rev/e7d30459a7f9 @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). 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}. 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. Integrated http://hg.netbeans.org/ergonomics/rev/dc417bdb3643 now let's see what gets broken. (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. Out of curiosity what is the performance now, with the new @MimeResolver.Registration ? No new slowness reports for 7.2Dev. Hopefully it is better. 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 (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. |