sdk icon indicating copy to clipboard operation
sdk copied to clipboard

[analyzer]: null issues when running Zapp (which embeds the analysis server on a web client)

Open Salakar opened this issue 6 months ago • 20 comments

Hello 👋

Firstly, I understand our use case is probably not one the Dart team will directly support but zapp.run has been working for some time on lots of Dart versions <= 2.19.2 - but something fundamentally has changed either in dart2js, JS interop or Analyzer LSP since Dart 3.

I've spent months trying to figure out why and try to boil it down to one small reproducible example but I've been unable to do so so I'm creating this issue with what I have currently and hoping someone can help out here either directly or suggesting patches I can apply to the Dart SDK that may fix it (I'm fully setup to patch and compile the Dart SDK locally).

Context: Zapp is a free-for-all open IDE to try out and easily share Dart / Flutter code with ~30k users and we'd love to keep it going but we're blocked and don't see any way forward, please help 🙏

Current issues:

file:///Users/mike/development/projects/services.zapp.run/packages/zapp_analyzer/lib/zapp_analyzer.dart:
Internal Error: The compiler crashed when compiling this element.

The compiler is broken.

When compiling the above element, the compiler crashed. It is not
possible to tell if this is caused by a problem in your program or
not. Regardless, the compiler should not crash.

The Dart team would greatly appreciate if you would take a moment to
report this problem at http://dartbug.com/new.

Please include the following information:

* the name and version of your operating system,

* the Dart SDK build number (3.2.4), and

* the entire message you see here (including the full stack trace
  below as well as the source location above).

The compiler crashed: Unsupported operation: RecordGetterData.forEachParameter
#0      RecordGetterData.forEachParameter (package:compiler/src/js_model/records.dart:496:5)
#1      JsKernelToElementMap.forEachParameter (package:compiler/src/js_model/element_map_impl.dart:1694:10)
#2      JsElementEnvironment.forEachParameter (package:compiler/src/js_model/element_map_impl.dart:2513:16)
#3      ProgramBuilder._computeParameterDefaultValues (package:compiler/src/js_emitter/program_builder/program_builder.dart:833:27)
#4      ProgramBuilder._buildMethod (package:compiler/src/js_emitter/program_builder/program_builder.dart:922:40)
#5      ProgramBuilder._buildClass.visitInstanceMember (package:compiler/src/js_emitter/program_builder/program_builder.dart:609:26)
#6      ProgramBuilder._buildClass.visitMember (package:compiler/src/js_emitter/program_builder/program_builder.dart:628:9)
#7      List.forEach (dart:core-patch/growable_array.dart:416:8)
#8      ProgramBuilder._buildClass (package:compiler/src/js_emitter/program_builder/program_builder.dart:684:38)
#9      List.forEach (dart:core-patch/growable_array.dart:416:8)
#10     ProgramBuilder.buildProgram.<anonymous closure> (package:compiler/src/js_emitter/program_builder/program_builder.dart:199:15)
#11     _LinkedHashMapMixin.forEach (dart:collection-patch/compact_hash.dart:633:13)
#12     ProgramBuilder.buildProgram (package:compiler/src/js_emitter/program_builder/program_builder.dart:198:10)
#13     EmitterImpl.emitProgram.<anonymous closure> (package:compiler/src/js_emitter/startup_emitter/emitter.dart:185:29)
#14     CompilerTask.measureSubtask (package:compiler/src/common/tasks.dart:181:35)
#15     EmitterImpl.emitProgram (package:compiler/src/js_emitter/startup_emitter/emitter.dart:184:29)
#16     CodeEmitterTask.assembleProgram.<anonymous closure> (package:compiler/src/js_emitter/code_emitter_task.dart:131:26)
#17     CompilerTask.measure (package:compiler/src/common/tasks.dart:66:51)
#18     CodeEmitterTask.assembleProgram (package:compiler/src/js_emitter/code_emitter_task.dart:100:12)
#19     JsBackendStrategy.assembleProgram (package:compiler/src/js_model/js_strategy.dart:377:35)
#20     Compiler.runCodegenEnqueuer (package:compiler/src/compiler.dart:553:39)
#21     Compiler.runSequentialPhases (package:compiler/src/compiler.dart:763:27)
<asynchronous suspension>
#22     Compiler.runInternal.<anonymous closure> (package:compiler/src/compiler.dart:318:7)
<asynchronous suspension>
#23     Compiler.runInternal (package:compiler/src/compiler.dart:317:5)
<asynchronous suspension>
#24     Compiler.run.<anonymous closure> (package:compiler/src/compiler.dart:238:11)
<asynchronous suspension>
#25     compile.<anonymous closure> (package:compiler/compiler_api.dart:252:30)
<asynchronous suspension>
#26     compile.compilationDone (package:compiler/src/dart2js.dart:745:3)
<asynchronous suspension>
#27     main (package:compiler/src/dart2js.dart:1265:3)
<asynchronous suspension>

The code that's being compiled isn't doing anything detailed itself or using records directly, this is for zapp.run (online Flutter/Dart Sandbox IDE) and historically this has worked, since Dart 3 we've been unable to compile our web version of the Dart analyzer anymore (we're still stuck on Dart 2.19.2 😭 ) with various issues relating to dart2js internal changes, this being one of them.

Our code in zapp_analyzer.dart is a thin wrapper around analysis_server, it imports the following:

import 'dart:convert';
import 'dart:async';
import 'dart:io';
import 'dart:js_util';
import 'dart:js' as js;
import 'package:analyzer/src/generated/sdk.dart' as lsp;
import 'package:analysis_server/src/analytics/analytics_manager.dart' as lsp;
import 'package:unified_analytics/src/analytics.dart' as analytics;
import 'package:analysis_server/src/legacy_analysis_server.dart' as lsp;
import 'package:analysis_server/src/server/features.dart' as lsp;
import 'package:analysis_server/src/server/crash_reporting_attachments.dart'
    as lsp;
import 'package:analysis_server/src/lsp/lsp_analysis_server.dart' as lsp;
import 'package:analysis_server/lsp_protocol/protocol_custom_generated.dart'
    as lsp;

...and starting the LSP client with something like:

  final channel = BrowserLspCommunicationChannel();
  final resourceProvider = BrowserResourceProvider();
  final analysisServerOptions = lsp.AnalysisServerOptions();
  analysisServerOptions.useLanguageServerProtocol = true;
  analysisServerOptions.cacheFolder = '/zapp/.analyzer';
  analysisServerOptions.featureSet = lsp.FeatureSet(
    completion: enableCompletion,
    search: enableSearch,
  );
  analysisServerOptions.packagesFile =
      '/zapp/project/.dart_tool/package_config.json';
  final sdkManager = lsp.DartSdkManager('/zapp/sdk/bin/cache/dart-sdk');
  final crashReportingAttachmentsBuilder =
      lsp.CrashReportingAttachmentsBuilder.empty;
  final instrumentationService = BrowserLspInstrumentationService();
  final analyticsManager = lsp.AnalyticsManager(analytics.NoOpAnalytics());
  _server = lsp.LspAnalysisServer(
    channel,
    resourceProvider,
    analysisServerOptions,
    sdkManager,
    analyticsManager,
    crashReportingAttachmentsBuilder,
    instrumentationService,
  );
  await _server?.contextManager.setRoots(<String>[
    '/zapp/project',
  ], <String>[
    '/zapp/sdk',
    '/zapp/project/.zapp',
    '/zapp/pub'
  ]);

...and we're compiling as follows:

"$FLUTTER_SDK"/bin/cache/dart-sdk/bin/dart compile js \
  -o "$FLUTTER_SDK"/.zapp/js/za.js \
  --sound-null-safety \
  --enable-asserts \
  --no-source-maps \
  -O0 \
  --packages=$(pwd)/packages/zapp_analyzer/.dart_tool/package_config.json \
  $(pwd)/packages/zapp_analyzer/lib/zapp_analyzer.dart

...which errors with the above. However; if we then try compile with -O1 optimization level it then compiles to JS fine, but then we get a strange runtime issue once the analyzer has read all the files and starts analyzing:

image

Null check operator used on a null value
TypeError: Cannot read properties of null (reading 'toString')
    at DefaultValueResolver._executable$2 (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:309842:10)
    at DefaultValueResolver._default_value_resolver$_constructor$2 (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:309829:12)
    at tear_off.eval (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:2296:45)
    at Object._ContextExtension_forEach (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:58437:11)
    at DefaultValueResolver._interface$2 (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:309855:9)
    at tear_off.eval (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:2296:45)
    at Object._ContextExtension_forEach (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:58437:11)
    at DefaultValueResolver.resolve$0 (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:309816:11)
    at Linker._resolveDefaultValues$0 (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:315656:48)
    at eval (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:315478:27)

    at Object.wrapException (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:1955:43)
    at LibraryContext._throwLibraryCycleLinkException$3 (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:227089:15)
    at eval (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:227191:18)
    at _wrapJsFunctionForAsync_closure.eval [as $protected] (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:4829:15)
    at _wrapJsFunctionForAsync_closure.call$2 (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:75254:12)
    at _awaitOnObject_closure0.call$2 (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:75248:25)
    at _RootZone.runBinary$3$3 (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:78168:18)
    at _FutureListener.handleError$1 (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:76019:21)
    at _Future__propagateToListeners_handleError.call$0 (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:76369:49)
    at Object._Future__propagateToListeners (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:5128:77)

If we debug/inspect the line it errors in this JS version of DefaultValueResolver.executable() then we can see t1 is null:

image and this is the SDK source relating to that code above (./pkg/analyzer/lib/src/summary2/default_value_resolver.dart): image

Inspecting the element we see it relates to _Completer constructor from future_impl.dart from dart:async seemingly not being able to be resolved by this portion of the analyzer:

image image


Side note: if I comment out the throw in RecordGetterData.forEachParameter it then compiles as expected but we get back to the same runtime JS error as above - so these are most likely two issues. I've also tried at -O4 optimization level and levels in between and we get what looks to be the same runtime null error (albeit minified and harder to read):

TypeError: Cannot read properties of null (reading 'toString')
    at fiM.c6W (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:142860:3)
    at fiM.vXG (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:142855:6)
    at tear_off.eval (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:894:67)
    at Object.q1F (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:29238:63)
    at fiM.AwH (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:142864:3)
    at tear_off.eval (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:894:67)
    at Object.q1F (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:29238:63)
    at fiM.SP (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:142848:3)
    at OBZ.YcQ (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:146007:18)
    at eval (eval at workerLoadAnalyzer (http://localhost:3000/src/client/fs/analyzer.api.ts:138:5), <anonymous>:145941:3)

I've also tried compiler flags like --disable-native-live-type-analysis & --no-frequency-based-minification - on their own, together, etc and same output/issue.


Side note 2: the online compiler for Zapp is also built in the same way (a thin Dart wrapper on dev_compiler & front_end then compiled to JS via dart2js and ran in a web worker) - this is still working without issue in Dart v3+.


Side note 3: I've been trying this on practically every stable Dart version that has come out since and always facing the same issue, I have also been trying the betas but I'm unable to try the very latest Dart betas since there's an issue with the build tools so it can't be compiled at the moment:

image


  • Dart version and tooling diagnostic info (dart info)

Current version info below but any version really since v3.

If providing this information as part of reporting a bug, please review the information
below to ensure it only contains things you're comfortable posting publicly.

#### General info

- Dart 3.2.4 (stable) (Thu Dec 21 19:13:53 2023 +0000) on "macos_arm64"
- on macos / Version 14.1.1 (Build 23B81)
- locale is en-GB

#### Project info

- sdk constraint: '>=2.17.0 <4.0.0'
- dependencies: archive, gcloud, googleapis_auth, melos, path, shelf, shelf_cors_headers, shelf_router, shelf_static
- dev_dependencies: 
- elided dependencies: 5

#### Process info

|  Memory |  CPU | Elapsed time | Command line                                                                               |
| ------: | ---: | -----------: | ------------------------------------------------------------------------------------------ |
|   49 MB | 0.0% |     22:26:48 | dart .<path>/serve_sdk.dart                                                                |
|   19 MB | 0.0% |     22:55:16 | dart devtools --machine --allow-embedding                                                  |
| 1518 MB | 0.0% |     22:55:16 | dart language-server --protocol=lsp --client-id=VS-Code --client-version=3.78.2            |
|   24 MB | 0.0% |     22:27:05 | dart run melos:melos CLI_LAUNCHER_LAUNCH_CONTEXT {"d":"<path>/services.zapp.run"}} run --no-select run:sdk |
|   41 MB | 0.0% |     22:55:16 | flutter_tools.snapshot daemon                                                              |
|   10 MB | 0.0% |     22:27:05 | melos_dev.dart-3.2.0.snapshot run --no-select run:sdk                                      |
  • Whether you are using Windows, macOS, or Linux (if applicable)

N/A

  • Whether you are using Chrome, Safari, Firefox, Edge (if applicable)

N/A

Salakar avatar Dec 28 '23 13:12 Salakar

Right so I got it 'working' with some patches to the following (highlighted code = added by me):

  1. ./pkg/analyzer/lib/src/summary2/default_value_resolver.dart: image

That fixed the above issue (for some reason elementNodes[element] not finding the element inside getLinkingNode), but then a new issue came up where typeInference! was null in the below, so I added the highlighted code in the below patch:

  1. ./pkg/analyzer/lib/src/dart/element/element.dart:

image


As you can see with the above 2 patches the analyzer is now 'working' (doesn't analyze anything else except dart built-ins, e.g. Flutter doesn't exist/resolve) :

image


These 2 patches are obviously huge code-smells and working around a problem elsewhere which I still don't know what it is, but putting patches here incase it maybe gives some indication of what the underlying problem is.


Note: there's still something not working here, as above it now doesn't analyze anything else except dart core builtins, e.g. Flutter doesn't exist/resolve, seeing some weird paths in the analyzer, e.g. compare this build broken paths:

image

... with paths correct on Dart 2.19.2 build:

image

... maybe paths/libraries missing issue?

Salakar avatar Dec 28 '23 16:12 Salakar

There's a lot of information here, but thanks for collecting it all, with relevant source and all, as well as for Zapp :D

Since there are two primary issues here, I would consider splitting this into two issues if you could. That way the relevant parties for each can more easily identify what's relevant to them and the discussion can be more focused. That would also allow for a more clear resolution/stopping point for each issue as well.

parlough avatar Dec 28 '23 17:12 parlough

FWIW, dart2js has this: https://github.com/dart-lang/sdk/blob/a7e1e26446a0debcd54984afdf8d0de554469325/pkg/compiler/lib/src/js_model/records.dart#L490-L499

Your stack trace shows that this function can indeed be called via FunctionData.forEachParameter, so I think we can simply remove the throw: https://dart-review.googlesource.com/c/sdk/+/344380

fishythefish avatar Jan 02 '24 22:01 fishythefish

Thanks @fishythefish!

That fix landed in https://github.com/dart-lang/sdk/commit/ff9e8632decafc44d867036491d3eb334629ed4b, so now it seems like the next issues are some null values in the analyzer. They might be harder to identify without being able to debug, but I'll ping @scheglov in case he has a chance to check or if anything jumps out to him.

Your mentioned dart info says you're running with Dart 3.2.4. Is that the SDK version you're modifying as well? Could you also try with a build at HEAD that has the dart2js fix, so that any potential changes to the analyzer since then are accounted for?

parlough avatar Jan 03 '24 21:01 parlough

It seems that there is too much context. What code do you try to analyze, on which the analyzer crashes?

scheglov avatar Jan 03 '24 22:01 scheglov

It seems that there is too much context. What code do you try to analyze, on which the analyzer crashes?

@scheglov Sorry I know it's a lot of information 😓 to clarify it crashes on any code analyzed from a Flutter app to even a basic print hello world Dart main with no imports. My assumption (maybe incorrect) is that type inference is failing on Dart core libraries as they're returning null.


Since there are two primary issues here, I would consider splitting this into two issues if you could. That way the relevant parties for each can more easily identify what's relevant to them and the discussion can be more focused. That would also allow for a more clear resolution/stopping point for each issue as well.

Hey @parlough! Thanks for taking a look (it was great chatting in Prague too albeit briefly sorry!), good idea - since that first issue seems to have a fixed merged now should I maybe just edit original issue to remove context around that?


I am working on fully open sourcing Zapp & SDK builds in the meantime since it's probably the best way to provide a repro, but may take a few days since it's quite tightly coupled to a few things that would still make it difficult for anyone to clone and run even if open sourced.

Salakar avatar Jan 04 '24 10:01 Salakar

Well, given that the analyzer when running in Dart VM can analyze "hello world", it might be:

  1. Something that we wrote in the analyzer that does not follow the language specification, and so breaks when compiled to JavaScript.
  2. An issue in the compiler.
  3. An issue with integration.

The specific code change in _scopeFromElement hints that maybe there is some issue with hashCode of Elements. We do have some tricks there, but I cannot immediately say why this code would be problematic.

scheglov avatar Jan 04 '24 17:01 scheglov

The specific code change in _scopeFromElement hints that maybe there is some issue with hashCode of Elements. We do have some tricks there, but I cannot immediately say why this code would be problematic.

You may be right here, looking at hashCode it defers to source.hashCode which uses source.fullName which I'm guessing is the package uri, and based on the screenshot above which showed there's some uri/pathing issues so I'm guessing the fullName is different to fullName once compiled into JS, I'm not sure why though and what's changed since 2.16.x -> 3.x.x. We've had path issues before and had to add a @patch for Uri.script for JS environment - wonder if something similar

I'm debugging to confirm path issues, will report back.

image

Salakar avatar Jan 05 '24 08:01 Salakar

Ok so after @scheglov's comment I went debugging element hashCodes, the Element is in the nodes list but doesn't find it based on hashCode, so I added some debugging and manually did a find to find the node in the list (and it is there), here's the results:

image

id and hashCode are different but in this context I think we can ignore id, but given the hash codes are different it's not finding the element in the elementNodes list. Diving further to see what is used for hashCode on this particular Element and it seems to be ElementLocation, so I compared those, and I think I found the culprit:

image

...note in the screenshot that the element we're trying to find has an extra ElementLocation component, and so the hashCode ends up being different because of that. I don't know why it's got an extra component yet 🔍


Edit: I patched to remove the empty components (just as a test) and the hashCodes are matching now, however Linker.getLinkingNode (shown below) still can't find the elements, but, if I iterate elementNodes myself and match on hashCode myself I can find them. Maybe something to do with elementNodes being a Map.identity() 🤔 `

final Map<ElementImpl, ast.AstNode> elementNodes = Map.identity(); image


Edit 2: Patching to resolve elements by hashCode kinda works (but probably not properly matching up), again though; just core libraries only work, analyzer can't find pub packages, and also other weird things like the following are happening:

image

So most likely I've not found root cause of overall issue yet

Salakar avatar Jan 05 '24 11:01 Salakar

I'm not 100% sure, but the extra empty component in ElementLocation might be the name of the unnamed constructor.

Given the warning with main, with the knowledge that we use Set of elements for this warnings, it seems again to me that there is something wrong with Element and its hashCode and/or operator==.

scheglov avatar Jan 05 '24 17:01 scheglov

I'm not 100% sure, but the extra empty component in ElementLocation might be the name of the unnamed constructor.

I got side tracked here and patched to see if I could get further, I did and things got to the 'kinda working' state (including packages!): image

but of course it wasn't a valid fix or anything and me just debugging things and trying to understand what's going on better.

Given the warning with main, with the knowledge that we use Set of elements for this warnings, it seems again to me that there is something wrong with Element and its hashCode and/or operator==.

Yep, I'm pretty sure it's Element and hashCode issues, just struggling to figure out why still 😅 currently looking at js_helpers primitives for hashCode computation since there was a change there for Dart 3: https://github.com/dart-lang/sdk/commit/e8ce98ac780278af33a467b6d1c73f05320ce4a3#diff-f8fa0efbe96a9d8a5a08c9ba7d489a1a569b892c1a92f522cda4d6eae8b6d0d5 - probably unrelated

Salakar avatar Jan 05 '24 19:01 Salakar

FYI - since the dart2js issue is complete, I've updated this bug title and labels.

sigmundch avatar Jan 05 '24 20:01 sigmundch

Yep, I'm pretty sure it's Element and hashCode issues, just struggling to figure out why still 😅 currently looking at js_helpers primitives for hashCode computation since there was a change there for Dart 3: e8ce98a#diff-f8fa0efbe96a9d8a5a08c9ba7d489a1a569b892c1a92f522cda4d6eae8b6d0d5 - probably unrelated

I'm pretty much out of ideas now 😞 I also tried reverting a bunch of commits around JS interop/runtime changes between 2.19.2 and 3.0.0 in the following files:

  • https://github.com/dart-lang/sdk/blame/main/sdk/lib/_internal/js_runtime/lib/collection_patch.dart
  • https://github.com/dart-lang/sdk/blob/main/sdk/lib/_internal/js_runtime/lib/js_helper.dart (a bunch of hash code changes happened here)

but no joy. Open to suggestions

Salakar avatar Jan 12 '24 11:01 Salakar

Update:

Tried Flutter 3.19.0-0.1.pre (with Dart 3.3.0) this morning (https://github.com/dart-lang/sdk/issues/54075 was the cause of me not being able to build beta - deleting my .dart_tool directories above the SDK root dir fixed it 🤷).

The analyzer issue above still persists though and I've also noticed our compiler is now broken too on this latest Dart version (not investigated yet) so I feel like I'm fighting a losing battle here and not sure whether to continue with this anymore

Salakar avatar Jan 16 '24 11:01 Salakar

I don't think we can really help with this unless you provide some straightforward reproduction, e.g. extract the problematic code into a minimal project which could easily be run locally. That would at least make it possible for us to debug it - rather than look at screenshots and try to guess what is going wrong in such a huge code base (the answer is: anything and everything could have gone wrong).

If you depend on running analyzer compiled to JS (which, I must stress, is not something we directly support) the best way of approach this and save yourself a lot of headache in the future would be to invest in making analyzer test suite run compiled to JS. Then you could at least have a chance to catch and debug breakages on a smaller scale.

mraleph avatar Jan 29 '24 13:01 mraleph

Looking forward to a fully open sourced Zapp & SDK build. That will make it much easier for us to reproduce and debug the issue. One other thing you might consider is compiling your code to WASM rather than to JS. While Dart2JS is more mature, WASM is likely a much better long term fit for this use case due to improved performance and semantics that exactly match native Dart compilers. For example, with WASM you get identical number semantics with native Dart so you won't be impacted if the Dart analyzer depends on true 64 bit ints. https://github.com/dart-lang/sdk/blob/main/pkg/dart2wasm/README.md You can find some other background information on where dart2wasm works today on https://docs.flutter.dev/platform-integration/web/wasm

jacob314 avatar Feb 02 '24 01:02 jacob314

Kindly someone put this forward to the core team

fisforfaheem avatar Feb 15 '24 12:02 fisforfaheem

There is no reason to leave off-topic comments on this issue and spam people.

We can't do much unless we have open-source reproduction of the problem.

mraleph avatar Feb 15 '24 12:02 mraleph

Dear Team, found this website: https://flutlab.io/ They are doing same as this zapp.run is doing, and using latest flutter.

Kindly can you contact them for guidance if possible.

fisforfaheem avatar Feb 19 '24 08:02 fisforfaheem

Dear Team, found this website: https://flutlab.io/ They are doing same as this zapp.run is doing, and using latest flutter.

Kindly can you contact them for guidance if possible.

I think (maybe i'll be wrong) flutterlab is not open sourced.

Ken-Andre avatar Feb 19 '24 09:02 Ken-Andre

But you may contact their technical team for HELP, guidance.

On Mon, Feb 19, 2024 at 2:47 PM The Kyan @.***> wrote:

Dear Team, found this website: https://flutlab.io/ They are doing same as this zapp.run is doing, and using latest flutter.

Kindly can you contact them for guidance if possible.

I think (maybe i'll be wrong) flutterlab is not open sourced.

— Reply to this email directly, view it on GitHub https://github.com/dart-lang/sdk/issues/54468#issuecomment-1952070892, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIRXJSA3UNXUBWPT35IU7H3YUMNRRAVCNFSM6AAAAABBFRFG52VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNJSGA3TAOBZGI . You are receiving this because you commented.Message ID: @.***>

fisforfaheem avatar Feb 19 '24 13:02 fisforfaheem

@Salakar

I played around with getting the devcompiler to run when compiled with dart2js. I ended up with weird problems and bisected the thing to a commit that increased the number of bits used in a bitfield backed by an integer. So the problem is lack of 64 bit integers when compiling with dart2js.

I fixed this in a patch, shared here: https://gist.github.com/jonasfj/b3a12dec0daf578f4396e762d35693f3

I didn't really try to land it in the SDK. But given how cheap it is, I don't see any harm to doing so. Though we might have to ask someone who owns this part of the code.

In particular the problem is probably: https://gist.github.com/jonasfj/b3a12dec0daf578f4396e762d35693f3#file-gistfile0-txt-L82-L119

And the behavior is probably very hard to debug by any other means than simply doing a bisection :rofl:

jonasfj avatar Mar 05 '24 17:03 jonasfj

The patch I have above is based on 4722e4decae290d64bb1aa63d7b39756e79cbae0, it's possible that other issues have arised since then.

jonasfj avatar Mar 05 '24 17:03 jonasfj

Interesting, why is it for 30 bits, not 52 or 53, whatever weird number of bits JavaScript uses for int?

scheglov avatar Mar 05 '24 17:03 scheglov

@jonasfj nice detective work! makes sense.

@scheglov in JavaScript bitwise operations are performed on 32-bit integers, but the rest of the arithmetic is performed on doubles. This means you can use 53 mantissa bits to represent integers precisely - giving range $[-(2^{53}-1), (2^{53}-1)]$ - and arithmetic operations on these integers will be precise as well (as long as you stay within the range), but bitwise operations are limited to 32-bits.

mraleph avatar Mar 05 '24 18:03 mraleph

https://dart-review.googlesource.com/c/sdk/+/355823

scheglov avatar Mar 05 '24 19:03 scheglov

@mraleph informed me that (int, int) is expensive.

And indeed, here is statistics for running over 6 copies of Flutter. With using int as the baseline.

All results
--------------------------------
flutter_elements
  reachableObjects
    count: 13092978
      change: 10.49% 1243212
    size: 1002486783 = 978990 KB = 956.0 MB
      change: 4.15% 39909944 = 38974 KB = 38.1 MB
  _SimpleUri
    count: 3126
    size(shallow): 250080 = 244 KB
    duplicateCount: 0
  InterfaceTypeImpl
    count: 158092
    size(shallow): 10117888 = 9880 KB = 9.6 MB
  LinkedData
    count: 0

I think we don't want to pay the price of using 10% more objects, and 4% more heap for a rare JavaScript case. So, I will revert this change.

scheglov avatar Mar 05 '24 23:03 scheglov

I think we don't want to pay the price of using 10% more objects, and 4% more heap for a rare JavaScript case. So, I will revert this change.

@scheglov

I think you don't need to store it in 2 int (2x 30 bits) to avoid a JS issue. You just need to use 2 int (2x 30 bits) while you perform the bitwise operations (in the JS implementation) and then store it using a normal int, which will be 64 bits or 53 bits depending on the platform. I believe that a limit of 53 constants (53 bits) is enough and won't change the current number of objects or memory utilization.

Note that the heap utilization increases because a Record is a new object/struct with 2 ints + a reference to it. The current implementation uses just an int, which has the same cost as a reference to an object.

gmpassos avatar Mar 05 '24 23:03 gmpassos

Dirty hack to only use (int, int) when dart.library.js is defined.

https://dart-review.googlesource.com/c/sdk/+/366822

We could probably make it even simpler by using BigInt :rofl:

Another option is to make the whole EnumSet a mixin, or just implement it direct on ElementImpl, which could then have two int properties. Then you wouldn't be paying for an extra object.

But conditional imports is not that bad either.

jonasfj avatar May 16 '24 14:05 jonasfj

Fixed in https://github.com/dart-lang/sdk/commit/160b9eadb377c9fcbbf9dc65f495c0e8d2ca8c8c

jonasfj avatar May 22 '24 10:05 jonasfj