Task #12229 (closed)
Opened 10 years ago
Closed 9 years ago
Bug: OmeroCpp: Fails to link on Windows with Ice 3.4
Reported by: | rleigh | Owned by: | rleigh |
---|---|---|---|
Priority: | critical | Milestone: | 5.1.0-m4 |
Component: | OmeroCpp | Version: | 5.0.1 |
Keywords: | n.a. | Cc: | John.Webber@…, cpp@… |
Resources: | n.a. | Referenced By: | n.a. |
References: | n.a. | Remaining Time: | n.a. |
Sprint: | n.a. |
Description
See:
- http://lists.openmicroscopy.org.uk/pipermail/ome-users/2014-April/004393.html
- http://ci.openmicroscopy.org/view/Experimental/job/OMERO-5.1-merge-win/54/consoleText
LINK : fatal error LNK1189: library limit of 65535 objects exceeded
We need to know why we have such as vast quantity of exported symbols in the DLL. Does the above build log or the contents of the workspace in http://ci.openmicroscopy.org/view/Experimental/job/OMERO-5.1-merge-win/ws/src/components/tools/OmeroCpp/target/omero/ provide any clue?
Thoughts:
- Ice 3.4 may be exporting more symbols in the slice-generated source than Ice 3.3
- The model objects may have increased in number and complexity
- Or a combination of the two may have pushed us over the limit
Still, the number is absurdly high. There's no way anyone would want or need to use all these entry points, so hopefully they can be reduced whatever the cause.
Attachments (10)
Change History (37)
comment:1 Changed 10 years ago by jamoore
- Cc John.Webber@… cpp@… added; jamoore John.Webber@… removed
- Milestone changed from Unscheduled to 5.0.2
- Owner set to rleigh
- Priority changed from major to critical
comment:2 Changed 10 years ago by rleigh
comment:3 Changed 10 years ago by rleigh
comment:4 Changed 10 years ago by rleigh
Additional data. OmeroCpp on Linux:
-rwxrwxr-x 1 rleigh rleigh 216831423 Apr 28 11:46 libomero_client.so.orig -rwxrwxr-x 1 rleigh rleigh 76557016 Apr 28 11:46 libomero_client.so.stripped % nm -D libomero_client.so.orig | wc -l 155988
The shared library is absurdly large, and the number of symbol exports is equally excessive. (By way of comparison, the largest library I can find is QtWebKit? which is 35MiB, and that's an entire web browser engine. The next largest are 10MiB and even these are exceptions.) Summary of exported types:
% nm -D libomero_client.so.orig | sed -e "s;.*\( . \).*;\1;" | sort | uniq -c 1091 B 1 D 51685 T 459 U 21230 V 6 w 81516 W
The weak symbols are all Ice ctors/dtors and other inlined methods and the vague symbols are all typeinfo objects. The text symbols are all generated Ice methods. So it looks like the bloat here is due to the sheer quantity of "stuff" slice2cpp is generating.
Assuming that this is the same for Windows, and it's unlikely to be significantly different other than how it implements typeinfo and other internal details, the symbol count is way over the 64k limit imposed by the Windows PE-COFF 16-bit symbol ordinals.
Microsoft say that there is no way to increase the limit, and you must split the DLL into a number of separate DLLs to circumvent it.
Regards,
Roger
comment:5 Changed 10 years ago by jamoore
There are certainly some objects we could strip out Roger if you think we could reach the limit that way. All the Prx objects for omero::model for example. If that doesn't do it, libomero_model and libomero_api here we come.
comment:6 Changed 10 years ago by rleigh
Crude breakdown of model symbol counts using the above file:
model: 66388 api: 62967 other: 26633
Note grep for ::api and model:: includes api:: templates specialised for model:: classes; these are included only in the api:: count.
Both model and api are dangerously close to the 16-bit limit. However, the counts may be different on Windows. If we split them, we will need to ensure that no circular dependencies are introduced.
comment:7 Changed 10 years ago by rleigh
Model symbols. Note that the numbers and total are overestimates (by ~1.5×) since some symbols are duplicated between categories.
% for o in $(grep "vtable for omero::model" /tmp/omero-client-symbols.txt | sed -e "s;.*::\(.*\);\1;" | sort); do printf "%6d %s\n" "$(egrep "$o(\$|::|>)" /tmp/omero-client-symbols.txt | wc -l)" "$o"; done 136 AcquisitionMode 55 AcquisitionModeI 3542 Annotation 586 AnnotationAnnotationLink 67 AnnotationAnnotationLinkI 120 Arc 95 ArcI 136 ArcType 55 ArcTypeI 108 BasicAnnotation 136 Binning 55 BinningI 120 BooleanAnnotation 127 BooleanAnnotationI 1210 Channel 396 ChannelAnnotationLink 67 ChannelAnnotationLinkI 530 ChannelBinding 119 ChannelBindingI 338 ChannelI 306 ChecksumAlgorithm 55 ChecksumAlgorithmI 494 CodomainMapContext 104 CommentAnnotation 127 CommentAnnotationI 136 ContrastMethod 55 ContrastMethodI 168 ContrastStretchingContext 83 ContrastStretchingContextI 136 Correction 55 CorrectionI 1268 Dataset 396 DatasetAnnotationLink 67 DatasetAnnotationLinkI 217 DatasetI 451 DatasetImageLink 67 DatasetImageLinkI 200 DBPatch 85 DBPatchI 331 Details 39 DetailsI 518 Detector 119 DetectorI 264 DetectorSettings 101 DetectorSettingsI 136 DetectorType 55 DetectorTypeI 430 Dichroic 83 DichroicI 136 DimensionOrder 55 DimensionOrderI 120 DoubleAnnotation 127 DoubleAnnotationI 184 Ellipse 227 EllipseI 549 Event 123 EventI 386 EventLog 73 EventLogI 215 EventType 55 EventTypeI 252 Experiment 1740 Experimenter 396 ExperimenterAnnotationLink 67 ExperimenterAnnotationLinkI 757 ExperimenterGroup 396 ExperimenterGroupAnnotationLink 67 ExperimenterGroupAnnotationLinkI 173 ExperimenterGroupI 199 ExperimenterI 97 ExperimentI 136 ExperimentType 55 ExperimentTypeI 184 ExternalInfo 73 ExternalInfoI 238 Family 55 FamilyI 120 Filament 95 FilamentI 136 FilamentType 55 FilamentTypeI 120 FileAnnotation 127 FileAnnotationI 728 Fileset 396 FilesetAnnotationLink 67 FilesetAnnotationLinkI 402 FilesetEntry 71 FilesetEntryI 227 FilesetI 424 FilesetJobLink 67 FilesetJobLinkI 256 FilesetVersionInfo 95 FilesetVersionInfoI 1015 Filter 205 FilterI 789 FilterSet 451 FilterSetEmissionFilterLink 67 FilterSetEmissionFilterLinkI 451 FilterSetExcitationFilterLink 67 FilterSetExcitationFilterLinkI 193 FilterSetI 136 FilterType 55 FilterTypeI 136 Format 55 FormatI 120 GenericExcitationSource 95 GenericExcitationSourceI 487 GroupExperimenterMap 73 GroupExperimenterMapI 136 Illumination 55 IlluminationI 2274 Image 396 ImageAnnotationLink 67 ImageAnnotationLinkI 309 ImageI 216 ImagingEnvironment 83 ImagingEnvironmentI 136 Immersion 55 ImmersionI 256 ImportJob 344 ImportJobI 104 IndexingJob 163 IndexingJobI 830 Instrument 235 InstrumentI 104 IntegrityCheckJob 163 IntegrityCheckJobI 1084 IObject 2054 Job 548 JobOriginalFileLink 67 JobOriginalFileLinkI 160 JobStatus 55 JobStatusI 448 Label 328 LabelI 248 Laser 143 LaserI 136 LaserMedium 55 LaserMediumI 136 LaserType 55 LaserTypeI 104 LightEmittingDiode 89 LightEmittingDiodeI 478 LightPath 396 LightPathEmissionFilterLink 67 LightPathEmissionFilterLinkI 416 LightPathExcitationFilterLink 67 LightPathExcitationFilterLinkI 171 LightPathI 406 LightSettings 77 LightSettingsI 452 LightSource 184 Line 227 LineI 13135 Link 2130 LinkI 104 ListAnnotation 121 ListAnnotationI 593 LogicalChannel 187 LogicalChannelI 120 LongAnnotation 127 LongAnnotationI 120 MapAnnotation 127 MapAnnotationI 310 Mask 239 MaskI 272 Medium 110 MediumI 120 MetadataImportJob 169 MetadataImportJobI 466 MicrobeamManipulation 103 MicrobeamManipulationI 136 MicrobeamManipulationType 55 MicrobeamManipulationTypeI 216 Microscope 83 MicroscopeI 136 MicroscopeType 55 MicroscopeTypeI 356 Namespace 396 NamespaceAnnotationLink 67 NamespaceAnnotationLinkI 139 NamespaceI 545 Node 396 NodeAnnotationLink 67 NodeAnnotationLinkI 163 NodeI 108 NumericAnnotation 542 Objective 125 ObjectiveI 200 ObjectiveSettings 77 ObjectiveSettingsI 978 OriginalFile 396 OriginalFileAnnotationLink 67 OriginalFileAnnotationLinkI 211 OriginalFileI 472 OTF 101 OTFI 120 ParseJob 169 ParseJobI 764 Path 380 PathI 409 Permissions 69 PermissionsI 136 PhotometricInterpretation 55 PhotometricInterpretationI 104 PixelDataJob 163 PixelDataJobI 2538 Pixels 396 PixelsAnnotationLink 67 PixelsAnnotationLinkI 369 PixelsI 451 PixelsOriginalFileMap 67 PixelsOriginalFileMapI 311 PixelsType 61 PixelsTypeI 626 PlaneInfo 396 PlaneInfoAnnotationLink 67 PlaneInfoAnnotationLinkI 163 PlaneInfoI 184 PlaneSlicingContext 89 PlaneSlicingContextI 1003 Plate 660 PlateAcquisition 396 PlateAcquisitionAnnotationLink 67 PlateAcquisitionAnnotationLinkI 169 PlateAcquisitionI 396 PlateAnnotationLink 67 PlateAnnotationLinkI 271 PlateI 249 Point 215 PointI 136 Polygon 209 PolygonI 136 Polyline 209 PolylineI 698 Project 396 ProjectAnnotationLink 67 ProjectAnnotationLinkI 451 ProjectDatasetLink 67 ProjectDatasetLinkI 169 ProjectI 136 Pulse 55 PulseI 278 QuantumDef 71 QuantumDefI 717 Reagent 396 ReagentAnnotationLink 67 ReagentAnnotationLinkI 181 ReagentI 200 Rect 233 RectI 1006 RenderingDef 167 RenderingDefI 148 RenderingModel 55 RenderingModelI 120 ReverseIntensityContext 65 ReverseIntensityContextI 1012 Roi 396 RoiAnnotationLink 67 RoiAnnotationLinkI 171 RoiI 763 Screen 396 ScreenAnnotationLink 67 ScreenAnnotationLinkI 223 ScreenI 451 ScreenPlateLink 67 ScreenPlateLinkI 124 ScriptJob 169 ScriptJobI 1657 Session 415 SessionAnnotationLink 67 SessionAnnotationLinkI 199 SessionI 978 Shape 986 Share 223 ShareI 160 ShareMember 67 ShareMemberI 200 StageLabel 77 StageLabelI 168 StatsInfo 65 StatsInfoI 104 TagAnnotation 127 TagAnnotationI 120 TermAnnotation 127 TermAnnotationI 144 TextAnnotation 658 Thumbnail 104 ThumbnailGenerationJob 163 ThumbnailGenerationJobI 83 ThumbnailI 120 TimestampAnnotation 127 TimestampAnnotationI 216 TransmittanceRange 83 TransmittanceRangeI 108 TypeAnnotation 120 UploadJob 169 UploadJobI 943 Well 396 WellAnnotationLink 67 WellAnnotationLinkI 255 WellI 451 WellReagentLink 67 WellReagentLinkI 664 WellSample 396 WellSampleAnnotationLink 67 WellSampleAnnotationLinkI 145 WellSampleI 104 XmlAnnotation 127 XmlAnnotationI ------ 102180
comment:8 Changed 10 years ago by rleigh
Working on how to split up the library, I've added splitting into separate omero-base, omero-api, omero-model and omero-client parts. I've done this on my cmake branch since it's easy to change things around. We can look at backporting to scons once it's working.
I've attached a few graphs of the internal dependencies. There's also a script which can generate different variants; the ones I've attached are heavily filtered to make things readable, omitting all omero-api and omero-model internal dependencies.
The outstanding problem is automatically determining which headers and sources in omero/ (excluding the subdirectories) are directly or indirectly dependent upon (or depending on) the model/api/cmd components. Some need putting in the base component (depended on by model/api) and some need putting in the client component (depending upon model and api). Since this applies to both the static and generated sources, it's not easy to solve. Ideally we would move them to appropriate paths to make it easy to distinguish, but I imagine we don't want to do this since it'll break things. Is this an option for 5.1?
Suggested split
base <- { model <- api } <- client <- { cmd } (not sure if it's worth splitting out)
comment:9 Changed 10 years ago by rleigh
- Status changed from new to accepted
Changed 10 years ago by rleigh
OmeroCpp dependencies (further simplified, stdlib and Ice deps filtered out)
comment:10 Changed 10 years ago by rleigh
@jamoore the https://trac.openmicroscopy.org.uk/ome/attachment/ticket/12229/noice-deps.pdf attachment is about the best overview I can create of the remaining problems to split the library. There are some unfortunate cycles in here (example: between client and model). Can you see any possibilities for removing these cycles?
Changed 10 years ago by rleigh
Changed 10 years ago by rleigh
comment:11 Changed 10 years ago by rleigh
The noice-deps-nogroup PDF shows the dependencies in an even more simplified form. This ungroups the omero components so they are placed in relation to their dependencies on the cmd/api/model/util modules.
comment:12 Changed 10 years ago by rleigh
Summary of dependency analysis:
Grouping cmd/api/model/util into separate libraries, we are looking at how the remaining components in omero/* relate to them so we can link everything together without cycles. Initially, I was considering a simple split into two pieces, "base" components which are depended upon by cmd/api/model/util and "toplevel" components which depend upon cmd/api/model/util/base. However, the knarly graph shows this isn't currently achievable.
- Base components
- Collections
- Constants
- FS
- ModelF
- ROMIO
- RTypes
- Scripts
- ServerErrors?
- System
- ServicesF
- templates
- Toplevel components
- all
- callbacks
- min
- SharedResources?
- Tables
- Problematic components
- API (used by util and toplevel, needs cmd)
- Repositories (used by api, needs cmd)
- RTypesI (used by model and toplevel, needs client/clientF/ObjectFactoryRegistrar)
- ClientErrors? (used by client, util and toplevel; could go in base by seems misplaced)
- client/clientF/ObjectFactoryRegistrar (should be toplevel, but have model dep via RTypesI)
So the split can be reduced to solving the above few problems. The model/RTypesI/ObjectFactoryRegistrar/Client loop is the most annoying, so if that can be broken, that's great. The others aren't cycles, so it's more a question of where they should live, or if some of the dependencies can be removed to allow them to move to the base or toplevel.
Roger
comment:13 Changed 10 years ago by rleigh
The "Shared state" at the end of RTypesI.h looks buggy. These variables are being defined in every translation unit. They should be declared extern in the header and defined in RTypesI.cpp so there's only a single definition... Or better, just define as local statics in the functions which return them since there is absolutely no need for them to be public or global. If this applies to other stuff, it will reduce some of the bloat a little.
comment:14 Changed 10 years ago by rleigh
https://github.com/openmicroscopy/openmicroscopy/pull/2435 opened for the RTypesI problem; not part of the bigger issue doing this split though.
comment:15 Changed 10 years ago by rleigh
Due to the (many) circular dependencies between -common and -api and -model, this splitting is not as simple as initially assumed. The dependency analysis only took into account header includes, and not the hundreds and hundreds of ice forward declarations. Unpicking all these dependencies isn't doable at the C++ level; these are all coming from the Ice slice interface definitions.
At this point: I have split up omero-client into omero-common, omero-model, omero-modelenums, omero-api and omero-client. However, while these compile and link correctly, the circular dependencies result in a large number of unresolved symbols in each library (particularly -common) which cause linker failures building the examples. The libraries aren't usable unless they are linkable, and this requires eliminating all these unresolved symbols. And none of this work addresses the core issue: the obscene symbol count and library size; it's only working around it.
I'm afraid that I don't have any solution to offer at present. The limits we're running into are hard limits built into the Windows linker, and there's no working around that, only reducing the symbol count by either splitting the library up or reducing the number of classes/methods we generate. The former doesn't seem possible at this time, and the latter will probably mean breaking the API. If anyone has any better ideas, please do mention them.
comment:16 Changed 10 years ago by jamoore
- Milestone changed from 5.0.2 to 5.0.3
We won't make 5.0.2 for this re-working. Pushing.
comment:17 Changed 10 years ago by rleigh
See attached cmake build logs. These identify DLL linkage issues with our C++ sources and headers (*not* the Ice generated sources). Possibly a result of moving to cmake if they depend on particular preprocessor defines being set during the build, but I would have expected this to affect the generated sources as well. These might possibly have an effect on the exported symbols.
There are also a number of warnings about the headers defining things multiple times, again in our sources, not the generated sources. These might also have some effect.
@jamoore, any idea what's at fault here, particularly for the dll inconsistecies which are always bugs? Missing explicit export/imports?
comment:18 Changed 10 years ago by jamoore
@rleigh, can you give me an example of what I'm looking for? (Might be easier to chat)
comment:19 Changed 10 years ago by rleigh
Just search for "inconsistent dll linkage" in the omerocpp-msbuild.log attachment.
comment:20 Changed 10 years ago by jamoore
rleigh, off-hand I don't know where these warnings are coming from. components/blitz/blitz_tools.py has:
if sys.platform == "win32": command.append("--dll-export") command.append("OMERO_API")
If that's not being passed, then that would likely be related. If you think these warnings pre-existed cmake, then we should go back to the scons log and compare.
comment:21 Changed 10 years ago by rleigh
That would make sense. cmake is providing its own define, which does the same thing and which could be used instead.
comment:22 Changed 10 years ago by rleigh
In the attached ice-graph-deps.zip, there are two scripts, ice-parse and ice-graph. In the openmicroscopy source tree, do a full build to generate all ice sources, and then (in the root) run ice-parse. This will scan and dump a complete dependency graph of all direct includes, forward declarations and definitions of all datatypes including classes, interfaces, structs, enums and exceptions with a full set of cross references. Run ice-graph to process this data into a set of graphviz graphs and SVGs. Note that due to the large number of nodes and edges, the graphs aren't very readable, so it has been split into a set of smaller graphs with a subset of the edges to show dependencies to and from the model, api and cmd parts. This is partially summarised in ice-internaldeps.txt which suggests some of the obvious fixes which require making, though it requires more thorough examination to untangle some of the more complex loops.
Note that it might be worth importing the dotfiles into other tools which can visualise them better than graphviz.
comment:23 Changed 10 years ago by rleigh
https://github.com/openmicroscopy/openmicroscopy/pull/2966 is a partial solution for this problem on develop/5.1. A similar solution could be used for 5.0.4 with scons.
This splits up omero-client into a second omero-ice library which contain only the slice-generated sources, leaving the concrete implementations in omero-client. This reduces the symbol count sufficiently to allow linking to work. However, due to still being dangerously close to the 65k threshold (120k/2 and the split won't be exactly half), we can't rely on this working with new Ice versions or new compilers/libraries which may emit more symbols and break again.
I'd like to suggest the following possibilities:
1) Separate the ice definitions into separate common/model/api/cmd/client components but without needing to make any breaking API changes in the concrete implementations other than adjusting includes. This is the minimally invasive approach, adjusting only what's strictly required (e.g. https://github.com/openmicroscopy/openmicroscopy/pull/2965), but has the disadvantage of still leaving a disorganised mess of dependencies--you won't be able to tell from the location of an ice file which library it belongs to--we'd need to hardcode this
2) Split into the same common/model/api/cmd/client components, but reorganise both the ice and source layouts to use common/model/api/cmd/client subdirectories, moving/splitting sources currently at the root into the appropriate component directory. This means that the components and dependencies are explicitly specified and easy to understand for both the end user and the build system making things massively simpler to maintain and use. This will remove the need to do such horrible graphing as above in order to understand how things work.
I previously thought we might need to split the model code, but by splitting the ice-generated code into a separate library, we split it in half anyway. With (2) we could end up with:
omero-client omero-api omero-ice-cmd omero-model omero-common omero-ice omero-ice-api omero-ice-cmd omero-ice-model omero-ice-common
with appropriate dependencies. The user can link with whichever are needed. We might not need to actually split into the many libraries--that's almost certainly overkill--but having the formal separation in the source tree would be useful to have, and will enable further splitting in the future as needed without having to break the API again. While I've suggested a common/model/api/cmd/client split above, which was discussed with josh earlier, other splits/naming might be possible/more optimal--the suggestions here are just that a formal split would be desirable; the nature of the split is certainly flexible.
comment:24 Changed 10 years ago by rleigh
To fix for 5.0.4, the scons build needs adjusting:
We have four categories of source:
- Static sources in components/tools/OmeroCpp/src
- Sources generated from combined.vm
- Sources generated from static ice files
- Sources generated from ice files generated from combined.vm
At present, 2, 3 and 4 are all stored in components/blitz/generated and then copied en masse to components/tools/OmeroCpp/target. However, we need to split out 3+4 so that they can be build and linked into libomero-ice separately from 1+2 which will be built and linked into libomero-client. The build system needs adjusting to put these into two different locations rather than the single one used at present. Once this is done, it should be fairly simple to make scons create two DLLs.
Changed 10 years ago by rleigh
comment:25 Changed 10 years ago by jamoore
I'd be for working through the naming of comment 23 and the split of comment 24 should be fairly doable via blitz/build.xml
comment:26 Changed 9 years ago by jamoore
Roger: what's the next step for this ticket?
comment:27 Changed 9 years ago by rleigh
- Resolution set to fixed
- Status changed from accepted to closed
I think everything on this ticket is addressed now. It was left open due to the test failures, but I think it can be closed since they are now mostly fixed and not really related to the link errors.
No change if I set RELEASE=Os: http://ci.openmicroscopy.org/view/Experimental/job/OMERO-5.1-merge-win/55/consoleText