Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Webdev: Look for pubspec.yaml next to .dart_tool/package_config.json instead of only current dir #2498

Merged
merged 4 commits into from
Sep 26, 2024

Conversation

sigurdm
Copy link
Contributor

@sigurdm sigurdm commented Sep 25, 2024

#2496

I think we should consider also doing #2497.

'.dart_tool',
'package_config.json',
);
if (File(candidate).existsSync()) break;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason not to use exists() here instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really - I believe I once learnt that the async overhead is a lot larger than doing the stat itself - but I doubt it makes any difference.

Do you prefer the async method?

Copy link
Contributor Author

@sigurdm sigurdm Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This little program seems to confirm it:

import 'dart:io';

main() async {
  final s = Stopwatch()..start();
  for (var i = 0; i < 10000; i++) {
    await File('abc').exists();
  }
  print('async ${s.elapsed}');
  s.reset();
  for (var i = 0; i < 10000; i++) {
    File('abc').existsSync();
  }
  print(' sync ${s.elapsed}');
}
> dart --version
Dart SDK version: 3.6.0-271.0.dev (dev) (Mon Sep 23 21:07:16 2024 -0700) on "linux_x64"
> dart bin/exists.dart
async 0:00:00.233795
 sync 0:00:00.019898

So it seems the sync methods are ~10 times faster!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, sync operations are definitely faster at the cost of blocking the thread from handling other asynchronous work. This is probably fine in this situation since I doubt we're doing any interesting asynchronous work while we're also looking for the package_config. Thanks for checking!

@sigurdm
Copy link
Contributor Author

sigurdm commented Sep 26, 2024

I'll land this - we can do the async switch in a follow-up if needed

@sigurdm sigurdm merged commit 017ab97 into dart-lang:main Sep 26, 2024
46 of 47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants